On Fri, Apr 20, 2012 at 3:07 AM, Richard Guenther
<richard.guent...@gmail.com> wrote:
> On Fri, Apr 20, 2012 at 10:41 AM, Richard Guenther
> <richard.guent...@gmail.com> wrote:
>> On Thu, Apr 19, 2012 at 8:51 PM, Xinliang David Li <davi...@google.com>
>> wrote:
>>> Compiling the attached test case with -O2 using the trunk compiler,
>>> the compiler will ICE. The proposed patch is also attached. The test
>>> is under going, but I like to have discussion on the right fix first.
>>
>> The patch doesn't look correct. There needs to be a VUSE, if not then
>> sth else is wrong.
>>
>> I'm looking into it.
>
> It is propagate_tree_value_into_stmt not properly updating SSA form. The
> following patch fixes that - but it looks bogus that PRE sees
great! thanks for the fix.
>
> t_12 = new 1000;
>
> as equivalent to t_12 = &g.
>
> Which is a separate issue. I suppose you can make a runtime testcase
> out of it and file a bug?
>
As you have noticed, this bogus transformation won't be executed at
runtime (either dead code or will trigger segfault).
thanks,
David
> Thanks,
> Richard.
>
>> Richard.
>>
>>> thanks,
>>>
>>> David
>>>
>>> Analysis:
>>> -------------
>>>
>>> Here is what is happening:
>>>
>>> BB6 : (Successor: BB8)
>>> MEM_19 = phi(MEM_24, MEM_25)
>>> t_9 = operator new (100)@MEM_19 (--> MEM_26)
>>> mem_ref(pp_6, -16).x@MEM_26 = t_9 (-->MEM_27) --> (1)
>>>
>>> BB8:
>>> ...
>>> MEM_20 = PHI(MEM_27, MEM_24)
>>> ....
>>> d.2348_15 = mem_ref(pp_6, -16).x@MEM_20
>>>
>>>
>>> In the loop header BB3 (which dominates BB6 and BB8),
>>>
>>> BB3:
>>> ..
>>> pp_31 = phi (pp_6, 0);
>>> ...
>>> pp_6 = pp_31 + 16
>>>
>>>
>>>
>>> In PRE, ANTIC_IN(BB8) includes mem_ref(pp_31,0),x@MEM_20. After phi
>>> translate, ANTIC_OUT(BB6) gets mem_ref(pp_31,0).x@MEM_27. However
>>> this expression gets propagated into ANTIC_IN(BB6) even though the
>>> memory expression is killed by (1). The root cause is that the alias
>>> oracle reports that mem_ref(pp_6, -16) and mem_ref(pp_31, 0) are not
>>> aliased as their points-to set do not intersect.
>>>
>>> As as consequence of the bad aliasing, the operator new calls gets
>>> transformed into an gimple assignment -- this gimple assignment
>>> becomes the defining statement for MEM_26. In a later UD chain walk,
>>> the memory chain stops (and triggers the assert) at this new
>>> assignment statement because there is no associated vuse for it.
>>>
>>>
>>> Test case
>>> --------------
>>>
>>> The case is synthesized -- the real world examples involve lots of inlining
>>> etc.
>>>
>>>
>>> int g, *gp[100];
>>> struct V {
>>> int* x;
>>> int y;
>>> };
>>>
>>> void foo (V **p, V* end, int i)
>>> {
>>> *p = 0;
>>> V* pp = *p;
>>> int s = 100;
>>> for (; pp < end; )
>>> {
>>> pp++;
>>> (pp-1)->x = &g;
>>> if (g)
>>> {
>>> if (g>10)
>>> g++;
>>> int *t = (int*) operator new (100);
>>> (pp-1)->x = t;
>>> }
>>> else
>>> s--;
>>> gp[end-pp] = (pp-1)->x + s;
>>> }
>>> }
>>>
>>>
>>> Patch
>>> ---------
>>>
>>>
>>> Index: tree-ssa-structalias.c
>>> ===================================================================
>>> --- tree-ssa-structalias.c (revision 186600)
>>> +++ tree-ssa-structalias.c (working copy)
>>> @@ -6137,6 +6137,9 @@ pt_solutions_intersect_1 (struct pt_solu
>>> if (pt1->anything || pt2->anything)
>>> return true;
>>>
>>> + if (pt1->null && pt2->null)
>>> + return true;
>>> +
>>> /* If either points to unknown global memory and the other points to
>>> any global memory they alias. */
>>> if ((pt1->nonlocal
