Re: Patch to fix compiler ICE due to bad PRE

[Richard Guenther <richard dot guenther at gmail dot com>]

On Fri, Apr 20, 2012 at 10:41 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Thu, Apr 19, 2012 at 8:51 PM, Xinliang David Li <davidxl@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?

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

Attachment: fix-propagate_tree_value_into_stmt
Description: Binary data