> -----Original Message----- > From: gcc-ow...@gcc.gnu.org [mailto:gcc-ow...@gcc.gnu.org] On Behalf Of > Richard Guenther > Sent: Tuesday, February 21, 2012 6:19 PM > To: Jiangning Liu > Cc: gcc@gcc.gnu.org > Subject: Re: A problem about loop store motion > > On Tue, Feb 21, 2012 at 10:57 AM, Jiangning Liu <jiangning....@arm.com> > wrote: > > > > > >> -----Original Message----- > >> From: Richard Guenther [mailto:richard.guent...@gmail.com] > >> Sent: Tuesday, February 21, 2012 5:40 PM > >> To: Jiangning Liu > >> Cc: gcc@gcc.gnu.org > >> Subject: Re: A problem about loop store motion > >> > >> On Tue, Feb 21, 2012 at 9:54 AM, Jiangning Liu > <jiangning....@arm.com> > >> wrote: > >> >> The MEM form is more "canonical", so the loop SM machinery to > detect > >> >> equality should be adjusted accordingly. Alternatively you can > >> teach > >> >> PRE insertion to strip off the MEM if possible (though > >> >> fold_stmt_inplace should > >> >> arelady do this if possible). > >> > > >> > Richard, > >> > > >> > Thank you! You are right. I noticed on latest trunk the problem in > >> PRE was > >> > already fixed by invoking fold_stmt_inplace. > >> > > >> > Unfortunately for this small case, the latest trunk code still > can't > >> do SM > >> > for variable pos, because refs_may_alias_p(*D.4074_10, pos) is > true, > >> that > >> > is, pos has alias with l[pos]. > >> > > >> > I think alias analysis should be able to know they don't have > alias > >> with > >> > each other, unless there is an assignment statement like "l=&pos;". > >> > > >> > Can alias analysis fix the problem? > >> > >> The problem is that 'pos' is marked TREE_ADDRESSABLE, so we think > >> its address got taken. 'l' points to any global possibly address- > taken > >> variable. It get's the TREE_ADDRESSABLE flag via PRE insertion > which > >> re-gimplifies the tree it creates which marks the variable as > >> addressable. > >> > >> I'll have a look. > > > > Terrific! :-) Could you please let me know when you have a patch to > fix > > this, because I want to double check the big case, and there might be > other > > hidden problems? > > PR52324, I am testing the following patch (in reality the gimplifier > should not > mark things addressable unless it itself makes them so, but frontends > are > broken, so we do that ... ugh). >
Richard, Now trunk works for the big case as well. Thanks a lot for your quick fix. BTW, why can't we fix front-end directly? Thanks, -Jiangning > Index: gcc/gimplify.c > =================================================================== > --- gcc/gimplify.c (revision 184428) > +++ gcc/gimplify.c (working copy) > @@ -7061,15 +7061,20 @@ gimplify_expr (tree *expr_p, gimple_seq > ret = GS_OK; > break; > } > - ret = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, > post_p, > - is_gimple_mem_ref_addr, fb_rvalue); > - if (ret == GS_ERROR) > - break; > + /* Avoid re-gimplifying the address operand if it is already > + in suitable form. */ > + if (!is_gimple_mem_ref_addr (TREE_OPERAND (*expr_p, 0))) > + { > + ret = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, > post_p, > + is_gimple_mem_ref_addr, fb_rvalue); > + if (ret == GS_ERROR) > + break; > + } > recalculate_side_effects (*expr_p); > ret = GS_ALL_DONE; > break; > > - /* Constants need not be gimplified. */ > + /* Constants need not be gimplified. */ > case INTEGER_CST: > case REAL_CST: > case FIXED_CST: