On Fri, Apr 20, 2012 at 12:07 PM, 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
>
> 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?
Somewhat simplified testcase that still performs the bogus replacement:
int g, *gp;
int *bar();
void foo (int ***p, int** end, int b)
{
*p = 0;
int** pp = *p;
for (;;)
{
pp++;
*pp = &g;
if (b)
{
if (b>1)
g = 0;
int *t = bar ();
*pp = t; <--- b
}
gp = *pp; <--- a
}
}
we note the bogus equivalency when trying to insert *pp from <--- a into
its predecessors. The translated expr in the if (b) block is
{mem_ref<8B>,pp_1}@.MEM_16 with a leader 't' which looks ok.
But then we are trying to insert *pp from <--- b into its predecessors.
Not sure why - but I suppose we removed 't' because it is in TMP_GEN
but left *pp as EXP_GEN. *pp ends up in ANTIC_IN because
clean does not consider *pp = t clobbering it - which is kind of correct - pp
points to NULL only, so storing into it cannot possibly alter what *pp points
to. Well, all of this is undefined territory, so if not ICEing then this is no
longer a bug (not wrong-code - just an interesting side-effect of
undefinedness).
Richard.
> 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
