| Message ID | 1651469.y9WBzJujMY@polaris |
|---|---|
| State | New |
| Headers | show |
On 12/06/2016 05:28 AM, Eric Botcazou wrote: > Hi, > > the compiler ICEs for SPARC 64-bit with LRA on the asm-subreg-1.c test: > > volatile unsigned short _const_32 [4] = {1,2,3,4}; > void > evas_common_convert_yuv_420p_601_rgba() > { > __asm__ __volatile__ ("" : : "X" (*_const_32)); > } > > The issue is that combine merges back the 3 instructions necessary to build > the address of _const_32 into a big MEM expression: > > (insn 10 9 0 2 (asm_operands/v ("") ("") 0 [ > (mem/v/c:HI (lo_sum:DI (mult:DI (lo_sum:DI (high:DI (unspec:DI [ > (symbol_ref:DI ("_const_32") [flags > 0x2] <var_decl 0x7ffff7ff6630 _const_32>) > ] UNSPEC_SETH44)) > (unspec:DI [ > (symbol_ref:DI ("_const_32") [flags 0x2] > <var_decl 0x7ffff7ff6630 _const_32>) > ] UNSPEC_SETM44)) > (const_int 4096 [0x1000])) > (symbol_ref:DI ("_const_32") [flags 0x2] <var_decl > 0x7ffff7ff6630 _const_32>)) [1 _const_32+0 S2 A16]) > ] > [ > (asm_input:HI ("X") asm-subreg-1.c:13) > ] > [] asm-subreg-1.c:13) "asm-subreg-1.c":13 -1 > (nil)) > > and LRA calls decompose_mem_address on the address, which aborts out of > confusion; reload (and all subsequent passes) lets it go through unmodified. > > The attached patch simply adds a bypass to process_address_1 in order to avoid > invoking decompose_mem_address in this case. > > Tested on SPARC/Solaris with LRA and x86-64/Linux, OK for the mainline? > > > 2016-12-06 Eric Botcazou <ebotcazou@adacore.com> > > * lra-constraints.c (process_address_1): Do not attempt to decompose > addresses for MEMs that satisfy fixed-form constraints. Presumably the MEM isn't a valid memory address, but it's allowed through due to the use of an "X" constraint? ISTM that LRA has to be prepared to handle an arbitrary RTX, so punting seems appropriate. jeff
> Presumably the MEM isn't a valid memory address, but it's allowed > through due to the use of an "X" constraint? Yes (that was supposed to be more or less clear given the description :-). > ISTM that LRA has to be prepared to handle an arbitrary RTX, so punting > seems appropriate. My opinion too. Note that Bernd E. has another solution though. -- Eric Botcazou
On 12/07/2016 04:21 PM, Eric Botcazou wrote: >> Presumably the MEM isn't a valid memory address, but it's allowed >> through due to the use of an "X" constraint? > > Yes (that was supposed to be more or less clear given the description :-). > >> ISTM that LRA has to be prepared to handle an arbitrary RTX, so punting >> seems appropriate. > > My opinion too. Note that Bernd E. has another solution though. > I wasn't ever able to get comfortable with the change in behavior that Bernd E's patch introduced. Namely that X no longer meant "anything", which is how it's been documented for eons. Jeff
> I wasn't ever able to get comfortable with the change in behavior that > Bernd E's patch introduced. Namely that X no longer meant "anything", > which is how it's been documented for eons. Understood. I have applied my patchlet then, with the other, more serious LRA patch I posted yesterday, it yields a clean C testsuite in 64-bit mode too. -- Eric Botcazou
Index: lra-constraints.c =================================================================== --- lra-constraints.c (revision 243245) +++ lra-constraints.c (working copy) @@ -3080,7 +3080,11 @@ process_address_1 (int nop, bool check_o if (insn_extra_address_constraint (cn)) decompose_lea_address (&ad, curr_id->operand_loc[nop]); - else if (MEM_P (op)) + /* Do not attempt to decompose arbitrary addresses generated by combine + for asm operands with loose constraints, e.g 'X'. */ + else if (MEM_P (op) + && !(get_constraint_type (cn) == CT_FIXED_FORM + && constraint_satisfied_p (op, cn))) decompose_mem_address (&ad, op); else if (GET_CODE (op) == SUBREG && MEM_P (SUBREG_REG (op)))
Hi, the compiler ICEs for SPARC 64-bit with LRA on the asm-subreg-1.c test: volatile unsigned short _const_32 [4] = {1,2,3,4}; void evas_common_convert_yuv_420p_601_rgba() { __asm__ __volatile__ ("" : : "X" (*_const_32)); } The issue is that combine merges back the 3 instructions necessary to build the address of _const_32 into a big MEM expression: (insn 10 9 0 2 (asm_operands/v ("") ("") 0 [ (mem/v/c:HI (lo_sum:DI (mult:DI (lo_sum:DI (high:DI (unspec:DI [ (symbol_ref:DI ("_const_32") [flags 0x2] <var_decl 0x7ffff7ff6630 _const_32>) ] UNSPEC_SETH44)) (unspec:DI [ (symbol_ref:DI ("_const_32") [flags 0x2] <var_decl 0x7ffff7ff6630 _const_32>) ] UNSPEC_SETM44)) (const_int 4096 [0x1000])) (symbol_ref:DI ("_const_32") [flags 0x2] <var_decl 0x7ffff7ff6630 _const_32>)) [1 _const_32+0 S2 A16]) ] [ (asm_input:HI ("X") asm-subreg-1.c:13) ] [] asm-subreg-1.c:13) "asm-subreg-1.c":13 -1 (nil)) and LRA calls decompose_mem_address on the address, which aborts out of confusion; reload (and all subsequent passes) lets it go through unmodified. The attached patch simply adds a bypass to process_address_1 in order to avoid invoking decompose_mem_address in this case. Tested on SPARC/Solaris with LRA and x86-64/Linux, OK for the mainline? 2016-12-06 Eric Botcazou <ebotcazou@adacore.com> * lra-constraints.c (process_address_1): Do not attempt to decompose addresses for MEMs that satisfy fixed-form constraints. -- Eric Botcazou