On Fri, Jul 22, 2011 at 10:16 AM, Richard Guenther
<richard.guent...@gmail.com> wrote:
> On Thu, Jul 21, 2011 at 7:21 PM, DJ Delorie <d...@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 <rguent...@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.
fix-pr45819
Description: Binary data
