On Thu, Jul 21, 2011 at 7:21 PM, DJ Delorie <[email protected]> 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.
Thanks,
Richard.
>
> <ssa_name 0x7f873559e3c8
> type <pointer_type 0x7f87354cfb28
> type <integer_type 0x7f87354bc540 unsigned int public unsigned SI
> size <integer_cst 0x7f87354a8708 constant 32>
> unit size <integer_cst 0x7f87354a8410 constant 4>
> align 32 symtab 0 alias set -1 canonical type 0x7f87354bc540
> precision 32 min <integer_cst 0x7f87354a8730 0> max <integer_cst
> 0x7f87354a86e0 4294967295>
> pointer_to_this <pointer_type 0x7f87354cfb28>>
> sizes-gimplified public unsigned SI size <integer_cst 0x7f87354a8708
> 32> unit size <integer_cst 0x7f87354a8410 4>
> align 32 symtab 0 alias set -1 canonical type 0x7f87354cfb28>
> var <var_decl 0x7f873559c000 status_reg>def_stmt status_reg_7 =
> &D.2010_2->port_status[wIndex.0_5]{lb: 0 sz: 4};
>
> version 7>
>
>
> <addr_expr 0x7f8735599750
> type <pointer_type 0x7f87354cfb28
> type <integer_type 0x7f87354bc540 unsigned int public unsigned SI
> size <integer_cst 0x7f87354a8708 constant 32>
> unit size <integer_cst 0x7f87354a8410 constant 4>
> align 32 symtab 0 alias set -1 canonical type 0x7f87354bc540
> precision 32 min <integer_cst 0x7f87354a8730 0> max <integer_cst
> 0x7f87354a86e0 4294967295>
> pointer_to_this <pointer_type 0x7f87354cfb28>>
> sizes-gimplified public unsigned SI size <integer_cst 0x7f87354a8708
> 32> unit size <integer_cst 0x7f87354a8410 4>
> align 32 symtab 0 alias set -1 canonical type 0x7f87354cfb28>
>
> arg 0 <array_ref 0x7f8735587510 type <integer_type 0x7f87354bc540 unsigned
> int>
>
> arg 0 <component_ref 0x7f873559d200 type <array_type 0x7f87355852a0>
>
> arg 0 <mem_ref 0x7f873b8cf460 type <record_type 0x7f87355850a8
> ehci_regs>
>
> arg 0 <ssa_name 0x7f873559e210 type <pointer_type
> 0x7f87355853f0>
> var <var_decl 0x7f873559c0a0 D.2010>def_stmt D.2010_2 =
> ehci_1(D)->regs;
>
> version 2>
> arg 1 <integer_cst 0x7f8735579618 constant 0>
> 45819.c:15:40> arg 1 <field_decl 0x7f87354b1b48 port_status>
> 45819.c:15:40>
> arg 1 <ssa_name 0x7f873559e318 type <integer_type 0x7f87354bc540
> unsigned int>
> var <var_decl 0x7f873559c1e0 wIndex.0>def_stmt wIndex.0_5 =
> (unsigned int) wIndex_4(D);
>
> version 5>
> arg 2 <integer_cst 0x7f87354a8c30 constant 0>>>
>
>
> <ssa_name 0x7f873559e420
> type <integer_type 0x7f8735585690 unsigned int volatile unsigned SI
> size <integer_cst 0x7f87354a8708 constant 32>
> unit size <integer_cst 0x7f87354a8410 constant 4>
> align 32 symtab 0 alias set -1 canonical type 0x7f8735585690 precision
> 32 min <integer_cst 0x7f87354a8730 0> max <integer_cst 0x7f87354a86e0
> 4294967295>
> pointer_to_this <pointer_type 0x7f8735585738>>
> var <var_decl 0x7f873559c3c0 D.2015>def_stmt D.2015_8 ={v} MEM[(volatile
> unsigned int *)status_reg_7];
>
> version 8>
>
>
> <mem_ref 0x7f873b8cf380
> type <integer_type 0x7f8735585690 unsigned int volatile unsigned SI
> size <integer_cst 0x7f87354a8708 constant 32>
> unit size <integer_cst 0x7f87354a8410 constant 4>
> align 32 symtab 0 alias set -1 canonical type 0x7f8735585690 precision
> 32 min <integer_cst 0x7f87354a8730 0> max <integer_cst 0x7f87354a86e0
> 4294967295>
> pointer_to_this <pointer_type 0x7f8735585738>>
> side-effects volatile
> arg 0 <ssa_name 0x7f873559e3c8
> type <pointer_type 0x7f87354cfb28 type <integer_type 0x7f87354bc540
> unsigned int>
> sizes-gimplified public unsigned SI size <integer_cst
> 0x7f87354a8708 32> unit size <integer_cst 0x7f87354a8410 4>
> align 32 symtab 0 alias set -1 canonical type 0x7f87354cfb28>
> var <var_decl 0x7f873559c000 status_reg>def_stmt status_reg_7 =
> &D.2010_2->port_status[wIndex.0_5]{lb: 0 sz: 4};
>
> version 7>
> arg 1 <integer_cst 0x7f8735579668 type <pointer_type 0x7f8735585738>
> constant 0>
> 45819.c:16:9>
>
