This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC: arm_legitimate_address_p change for minipool constants


> I have a testcase that, after backporting Richard's patch to constrain MEM
> operands more strictly after reload to the 3.3 branch, started ICEing. 
> Unfortunately I could not reproduce the problem with HEAD, but I'm pretty
> sure it's still there - I just can't get combine to generate the problematic
> zero_extend any more.
> 
> The test comes from glib and is a 64-bit byte swap.  Here's the troublesome
> insn before arm_reorg:
> 
> (insn:HI 49 193 209 2 0x4036d2ec (set (reg:DI 3 r3 [77])
>         (zero_extend:DI (mem/u/f:QI (symbol_ref/u:SI ("*.LC1")) [0 S1 A8]))) 146 {zero_extendqidi2}
>     (insn_list:REG_DEP_ANTI 208 (insn_list:REG_DEP_ANTI 207 (insn_list:REG_DEP_OUTPUT 61 (nil))))
>     (expr_list:REG_UNUSED (reg:SI 4 r4)
>         (nil)))
> 
> After arm_reorg:
> 
> (insn:HI 49 193 209 0x4036d2ec (set (reg:DI 3 r3 [77])
>         (zero_extend:DI (mem:QI (const (plus (label_ref 249)
>                         (const_int 52 [0x34]))) [0 S1 A8]))) 146 {zero_extendqidi2}
>     (insn_list:REG_DEP_ANTI 208 (insn_list:REG_DEP_ANTI 207 (insn_list:REG_DEP_OUTPUT 61 (nil))))
>     (expr_list:REG_UNUSED (reg:SI 4 r4)
>         (nil)))
> 
> Notice that the mem is QImode.  This is rejected:
>   else if (GET_MODE_SIZE (MODE) >= 4 && reload_completed                \
>            && (GET_CODE (X) == LABEL_REF                                \
>                || (GET_CODE (X) == CONST                                \
>                    && GET_CODE (XEXP ((X), 0)) == PLUS                  \
>                    && GET_CODE (XEXP (XEXP ((X), 0), 0)) == LABEL_REF   \
>                    && GET_CODE (XEXP (XEXP ((X), 0), 1)) == CONST_INT)))\
>     goto LABEL;                                                         \
> 
> If I remove the GET_MODE_SIZE check, the test passes.  I did some
> archaeology, to the limited extent I can from the gcc.gnu.org repository;
> the check has been there since revision 1.1.  I'm guessing it came in a
> patch from Richard Earnshaw in 1995 based on FSFChangeLog.11.  I can not see
> the reason for it, and the comment above it in arm_legitimate_address_p in
> HEAD explains all the other terms of the condition except for that one.
> 

You don't really expect me to be able to remember precisely why I put that 
test in over 8 years ago, surely :-)

I suspect that it was because I thought that such an access would ever 
really need to reference the constant pool (a byte-load of an immediate 
can just be replaced by a mov of the immediate) -- however, I'm no-longer 
convinced of that case: if code plays funny games with casts, then it is 
possible to force the compiler into making such a reference by accessing a 
subreg of a label or symbol.

> I have just started regression testing the following patch; OK if it passes?
> Can anyone think of a reason for the test to have been originally added?

I'm not sure if it's precisely the same problem, but have you looked at 
pulling up revision 1.302 of arm.c?

> -- 
> Daniel Jacobowitz
> MontaVista Software                         Debian GNU/Linux Developer
> 
> 2004-01-21  Daniel Jacobowitz  <drow@mvista.com>
> 
> 	* arm.c (arm_legitimate_address_p): Don't check the mode size
> 	for minipool references.
> 
> Index: arm.c
> ===================================================================
> RCS file: /big/fsf/rsync/gcc-cvs/gcc/gcc/config/arm/arm.c,v
> retrieving revision 1.303.2.15
> diff -u -p -r1.303.2.15 arm.c
> --- arm.c	15 Jan 2004 10:43:18 -0000	1.303.2.15

Erm, 1.303.2 is the csl-arm branch, not gcc-3.3.  What exactly are you 
asking for approval for?

R.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]