[Bug tree-optimization/100053] New: tree-fre incorrectly delete a condition

[Bug tree-optimization/100053] New: tree-fre incorrectly delete a condition

[qinzhao at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100053

            Bug ID: 100053
           Summary: tree-fre incorrectly delete a condition
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: qinzhao at gcc dot gnu.org
  Target Milestone: ---

hi, this is a bug with tree-fre optimization that caused run-time segmentation
fault. 
the original testing case cannot be posted.

the following is the reduced testing case through Creduce. 
ubuntu@qinzhao-ubuntu-x86:~/Bugs/32423691$ cat ksm.i
typedef a;
typedef struct b b;
struct {
  char c
} typedef d;
struct e {
  int f;
  char g
} typedef aa;
struct {
  int ad;
  char ae
} typedef ab;
struct {
  int ac;
  char ah;
  int ai;
  a *h;
  b *ag
} typedef af;
struct b {
  af i;
  struct e *j
} m;
k, o, q, r, s, t, u;
typedef l[];
l n;
*p;
ab al;
v(init) {
  d *aj;
  int ak;
  if (!init) {
    w();
    aj = q;
    if (p[t])
      goto aq;
    af am = (&m)->i;
    if (_setjmp())
      if ((&m)->i.h) {
        a an, ao;
        aa *ap = am.h[(&m)->i.ai];
        void *au;
        if ((&m)->i.ai) {
          if (x())
            an += (long)au % (&m)->i.ag->j->f;
          if ((a)__builtin_alloca)
            ao = au = an;
          ap->g = "";
        }
        y(ao);
      }
    ak |= z(n[k], ak, init ? 0 : ((int *)o)[t]);
    (&al)->ad = (&m)->i.ac & (&m)->i.ah;
    (&m)->i.ac = (&al)->ae = &al;
  }
  if (!init)
    aj->c = s;
  ar(u, r, s);
aq:
  if (!init)
    as(((int *)o)[t]);
}
ubuntu@qinzhao-ubuntu-x86:~/Bugs/32423691$ cat t
/home/ubuntu/Install/latest-debug/bin/gcc ksm.i -c -o ksm.o  -O2 
-fdump-tree-optimized 

ubuntu@qinzhao-ubuntu-x86:~/Bugs/32423691$ sh t
....

there are quite some warnings during compilation, please ignore them.

check the ksm.i.244t.optimized, you will find that the last "if (!init)" is
completely deleted;

if you add -fno-tree-fre to the compilation line, the last "if (!init)" will
not be deleted anymore.

[Bug tree-optimization/100053] tree-fre incorrectly delete a condition

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100053

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Removing unexecutable edge from if (init_101(D) == 0)
Merging blocks 25 and 26

[Bug tree-optimization/100053] tree-fre incorrectly delete a condition

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100053

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-04-13
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
I will have a look.

[Bug tree-optimization/100053] tree-fre incorrectly delete a condition

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100053

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
It would be nice if the reduced testcase could be sanitized to throw less
diagnostics with -Wall, likewise if it were a runtime testcase.

Reduced:

int __attribute__((returns_twice,noipa)) x() { return 0; }
void __attribute__((noipa)) ar() {}
void __attribute__((noipa)) as() { __builtin_abort (); }
int a1, a2, a3;
void __attribute__((noipa)) v(int init)
{ 
  if (!init) {
    as();
    if (a1)
      goto aq;
    if (x ())
      if (a2)
        as();
  }
  if (!init)
    a3 = 1;
  ar();
aq:
  if (!init)
    as();
}

int main()
{
  v(1);
  return 0;
}

[Bug tree-optimization/100053] tree-fre incorrectly delete a condition

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100053

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 50579
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50579&action=edit
fix for the issue

I am testing this patch - maybe you can test it on the original testcase you
cannot disclose.

Basically the VN iteration logic fails to make sure to re-evaluate predicated
values validity when backwards reachable edges become executable.  Fixing
that would be possible but simply not being optimistic for predicated values
is cheaper, predicated values are a somewhat oddly bolted on feature anyway.

[Bug tree-optimization/100053] [9/10/11 Regression] tree-fre incorrectly delete a condition

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100053

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |9.4
            Summary|tree-fre incorrectly delete |[9/10/11 Regression]
                   |a condition                 |tree-fre incorrectly delete
                   |                            |a condition
           Keywords|                            |wrong-code
      Known to work|                            |8.4.0
           Priority|P3                          |P2

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Btw, it's a regression that came in with the VN rewrite in GCC 9.

[Bug tree-optimization/100053] [9/10/11 Regression] tree-fre incorrectly delete a condition

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100053

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 50581
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50581&action=edit
improved fix to avoid regressions

This variant of the fix avoids regressing predication in the cases that work OK
and adds a testcase for one such case.  I filed PR100063 for the general
weakness
of predicate handling in VN.

[Bug tree-optimization/100053] [9/10/11 Regression] tree-fre incorrectly delete a condition

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100053

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:f9810422f6768b914aabfcbffe64f535bdd18452

commit r11-8152-gf9810422f6768b914aabfcbffe64f535bdd18452
Author: Richard Biener <rguenther@suse.de>
Date:   Tue Apr 13 12:05:53 2021 +0200

    tree-optimization/100053 - fix predication in VN

    This avoids doing optimistic dominance queries involving
    non-executable backedges when validating recorded predicated values
    in VN because we have no way to force re-evaluating validity when
    optimistically not executable edges become executable later.

    2021-04-13  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/100053
            * tree-ssa-sccvn.c (vn_nary_op_get_predicated_value): Do
            not use optimistic dominance queries for backedges to validate
            predicated values.
            (dominated_by_p_w_unex): Add parameter to ignore executable
            state on backedges.
            (rpo_elim::eliminate_avail): Adjust.

            * gcc.dg/torture/pr100053.c: New testcase.
            * gcc.dg/tree-ssa/ssa-fre-93.c: Likewise.

[Bug tree-optimization/100053] [9/10 Regression] tree-fre incorrectly delete a condition

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100053

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |11.0
            Summary|[9/10/11 Regression]        |[9/10 Regression] tree-fre
                   |tree-fre incorrectly delete |incorrectly delete a
                   |a condition                 |condition

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
Should be fixed on trunk.

[Bug tree-optimization/100053] [9/10 Regression] tree-fre incorrectly delete a condition

[qinzhao at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100053

--- Comment #9 from qinzhao at gcc dot gnu.org ---
(In reply to Richard Biener from comment #3)
> It would be nice if the reduced testcase could be sanitized to throw less
> diagnostics with -Wall, likewise if it were a runtime testcase.
> 
> Reduced:
> 
> int __attribute__((returns_twice,noipa)) x() { return 0; }
> void __attribute__((noipa)) ar() {}
> void __attribute__((noipa)) as() { __builtin_abort (); }
> int a1, a2, a3;
> void __attribute__((noipa)) v(int init)
> { 
>   if (!init) {
>     as();
>     if (a1)
>       goto aq;
>     if (x ())
>       if (a2)
>         as();
>   }
>   if (!init)
>     a3 = 1;
>   ar();
> aq:
>   if (!init)
>     as();
> }
> 
> int main()
> {
>   v(1);
>   return 0;
> }

Hi, thanks for the further reduced testing case.
I am wondering whether you did the above further reducing manually?

[Bug tree-optimization/100053] [9/10 Regression] tree-fre incorrectly delete a condition

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100053

--- Comment #10 from rguenther at suse dot de <rguenther at suse dot de> ---
On April 13, 2021 5:28:37 PM GMT+02:00, "qinzhao at gcc dot gnu.org"
<gcc-bugzilla@gcc.gnu.org> wrote:
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100053
>
>--- Comment #9 from qinzhao at gcc dot gnu.org ---
>(In reply to Richard Biener from comment #3)
>> It would be nice if the reduced testcase could be sanitized to throw
>less
>> diagnostics with -Wall, likewise if it were a runtime testcase.
>> 
>> Reduced:
>> 
>> int __attribute__((returns_twice,noipa)) x() { return 0; }
>> void __attribute__((noipa)) ar() {}
>> void __attribute__((noipa)) as() { __builtin_abort (); }
>> int a1, a2, a3;
>> void __attribute__((noipa)) v(int init)
>> { 
>>   if (!init) {
>>     as();
>>     if (a1)
>>       goto aq;
>>     if (x ())
>>       if (a2)
>>         as();
>>   }
>>   if (!init)
>>     a3 = 1;
>>   ar();
>> aq:
>>   if (!init)
>>     as();
>> }
>> 
>> int main()
>> {
>>   v(1);
>>   return 0;
>> }
>
>Hi, thanks for the further reduced testing case.
>I am wondering whether you did the above further reducing manually?

Yes, I did.

[Bug tree-optimization/100053] [9/10 Regression] tree-fre incorrectly delete a condition

[qinzhao at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100053

--- Comment #11 from qinzhao at gcc dot gnu.org ---
(In reply to Richard Biener from comment #4)
> Created attachment 50579 [details]
> fix for the issue
> 
> I am testing this patch - maybe you can test it on the original testcase you
> cannot disclose.
>

I tested this patch with the original test case, and it resolved the runtime
error.
thanks.

[Bug tree-optimization/100053] [9/10 Regression] tree-fre incorrectly delete a condition

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100053

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:be0093e7273f00fe850578415c0b06bc7dec6dc0

commit r10-9769-gbe0093e7273f00fe850578415c0b06bc7dec6dc0
Author: Richard Biener <rguenther@suse.de>
Date:   Tue Apr 13 12:05:53 2021 +0200

    tree-optimization/100053 - fix predication in VN

    This avoids doing optimistic dominance queries involving
    non-executable backedges when validating recorded predicated values
    in VN because we have no way to force re-evaluating validity when
    optimistically not executable edges become executable later.

    2021-04-13  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/100053
            * tree-ssa-sccvn.c (vn_nary_op_get_predicated_value): Do
            not use optimistic dominance queries for backedges to validate
            predicated values.
            (dominated_by_p_w_unex): Add parameter to ignore executable
            state on backedges.
            (rpo_elim::eliminate_avail): Adjust.

            * gcc.dg/torture/pr100053.c: New testcase.
            * gcc.dg/tree-ssa/ssa-fre-93.c: Likewise.

    (cherry picked from commit f9810422f6768b914aabfcbffe64f535bdd18452)

[Bug tree-optimization/100053] [9 Regression] tree-fre incorrectly delete a condition

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100053

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:6f2a65357f7b122a4e2b78b235d6faaa3d7ab3a4

commit r9-9509-g6f2a65357f7b122a4e2b78b235d6faaa3d7ab3a4
Author: Richard Biener <rguenther@suse.de>
Date:   Tue Apr 13 12:05:53 2021 +0200

    tree-optimization/100053 - fix predication in VN

    This avoids doing optimistic dominance queries involving
    non-executable backedges when validating recorded predicated values
    in VN because we have no way to force re-evaluating validity when
    optimistically not executable edges become executable later.

    2021-04-13  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/100053
            * tree-ssa-sccvn.c (vn_nary_op_get_predicated_value): Do
            not use optimistic dominance queries for backedges to validate
            predicated values.
            (dominated_by_p_w_unex): Add parameter to ignore executable
            state on backedges.
            (rpo_elim::eliminate_avail): Adjust.

            * gcc.dg/torture/pr100053.c: New testcase.
            * gcc.dg/tree-ssa/ssa-fre-93.c: Likewise.

    (cherry picked from commit f9810422f6768b914aabfcbffe64f535bdd18452)

[Bug tree-optimization/100053] [9 Regression] tree-fre incorrectly delete a condition

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100053

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
      Known to fail|                            |9.3.0
      Known to work|                            |9.3.1

--- Comment #14 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.