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. 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> >