PR 45819 - possible fix?

[Richard Guenther]

On Fri, Jul 22, 2011 at 10:16 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Thu, Jul 21, 2011 at 7:21 PM, DJ Delorie <dj@redhat.com> wrote:
>>
>>> The patch is not correct, it papers over a problem elsewhere (maybe
>>> in forwprop).
>>>
>>> I can't btw reproduce the issue on either the 4.5 or the 4.6 branch or trunk
>>> with the testcase from the PR.  I always get
>>>
>>>   v_2 ={v} st_1(D)->ptr;
>>>
>>> in the .optimized tree dump which correctly states this is a volatile load.
>>
>> x86 isn't strict_align so the original problem doesn't happen.  With
>> armv7hl-redhat-linux-gnueabi it happens with this test case (from the
>> PR):
>>
>> struct ehci_regs {
>> char x;
>> unsigned int port_status[0];
>> } __attribute__ ((packed));
>> //} __attribute__ ((packed,aligned(__alignof__(int))));
>>
>> struct ehci_hcd{
>> struct ehci_regs *regs;
>> };
>>
>> int ehci_hub_control (
>>  struct ehci_hcd *ehci,
>>  int wIndex
>> ) {
>>  unsigned int *status_reg = &ehci->regs->port_status[wIndex];
>>  return *(volatile unsigned int *)status_reg;
>> }
>>
>> Here's the 45819.c.023t.ccp1 dump:
>>
>> ;; Function ehci_hub_control (ehci_hub_control)
>>
>> ehci_hub_control (struct ehci_hcd * ehci, int wIndex)
>> {
>>  unsigned int * status_reg;
>>  volatile unsigned int D.2015;
>>  int D.2014;
>>  unsigned int D.2013;
>>  unsigned int wIndex.0;
>>  unsigned int[0:] * D.2011;
>>  struct ehci_regs * D.2010;
>>
>> <bb 2>:
>>  D.2010_2 = ehci_1(D)->regs;
>>  D.2011_3 = &D.2010_2->port_status;
>>  wIndex.0_5 = (unsigned int) wIndex_4(D);
>>  D.2013_6 = wIndex.0_5 * 4;
>>  status_reg_7 = D.2011_3 + D.2013_6;
>>  D.2015_8 ={v} MEM[(volatile unsigned int *)status_reg_7];
>>  D.2014_9 = (int) D.2015_8;
>>  return D.2014_9;
>>
>> }
>>
>> The problem happens when status_reg_7 is replaced in the D.2015_9
>> assignment.  I've attached the relevent before/after lhs/rhs trees.
>> By replacing the address with the previously computed value, gcc seems
>> to end up with alignment info that the user was trying to get rid of,
>> so gcc produces four byte-loads instead of a single word-load.
>>
>> 024t.forwprop1 shows this:
>>
>>  D.2015_8 = MEM[(volatile unsigned int *)D.2010_2].port_status[wIndex.0_5]{lb: 0 sz: 4};
>>
>> It looks like the "volatile unsigned int *" semantics now happen
>> *before* the structure reference semantics, instead of after, and the
>> alignment of the structure field takes precedence over the volatile,
>> instead of the other way around.  At least, that's what it looks like
>> to me ;-)
>
> No, that looks simply like a forwprop bug - I'll have a look.

I'm testing the following.

Richard.

2011-07-22  Richard Guenther  <rguenther@suse.de>

        PR tree-optimization/45819
        * tree-ssa-forwprop.c (forward_propagate_addr_expr_1): Properly
        preserve volatile and notrap flags.

        * gcc.dg/pr45819.c: New testcase.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-pr45819
Type: application/octet-stream
Size: 3063 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20110722/bb604907/attachment.obj>