On Wed, Jul 28, 2021 at 4:39 PM Andrew MacLeod <amacl...@redhat.com> wrote:
>
> So Im seeing what appears to me to be inconsistent behaviour.
>
> in pr96094.c we see:
>
> int
> foo (int x)
> {
> if (x >= 2U)
> return 34;
> return 34 / x;
> }
>
> x has a range of [0,1] and since / 0 in undefined, the expectation is
> that we fold this to "return 34" and vrp1 does this:
>
> <bb 3> [local count: 767403281]:
> x_6 = ASSERT_EXPR <x_3(D), (unsigned int) x_3(D) <= 1>;
> _4 = 34 / x_6;
>
> <bb 4> [local count: 1073741824]:
> # _2 = PHI <34(2), _4(3)>
>
> Transformed to
>
> <bb 3> [local count: 767403281]:
> _4 = 34;
>
> <bb 4> [local count: 1073741824]:
> # _2 = PHI <34(2), _4(3)>
>
>
> but if we go to tree-ssa/pr61839_2.c, we see:
>
> volatile unsigned b = 1U;
> int c = 1;
> c = (a + 972195718) % (b ? 2 : 0);
> if (c == 1)
> ;
> else
> __builtin_abort ();
>
> /* Dont optimize 972195717 / 0 in function foo. */
> /* { dg-final { scan-tree-dump-times "972195717 / " 1 "evrp" } } */
>
>
> So why is it OK to optimize out the divide in the first case, but not in
> the second??
>
> Furthermore, If I tweak the second testcase to:
>
> int a = -1;
> volatile unsigned b = 1U;
> int c = 1;
> c = (a + 972195718) / (b ? 2 : 0);
> if (c == 486097858)
> ;
> else
> __builtin_abort ();
>
> int d = 1;
> d = (a + 972195718) / (b ? 1 : 0);
> if (d == 972195717)
> ;
> else
> __builtin_abort ();
> return d;
>
> NOte the only difference is the first case divides by 0 or 2, the second
> case by 0 or 1...
>
> we quite happily produce:
>
> <bb 2> :
> b ={v} 1;
> b.1_2 ={v} b;
> if (b.1_2 != 0)
> goto <bb 4>; [INV]
> else
> goto <bb 3>; [INV]
>
> <bb 3> :
>
> <bb 4> :
> # iftmp.0_7 = PHI <2(2), 0(3)>
> c_14 = 972195717 / iftmp.0_7;
> if (c_14 == 486097858)
> goto <bb 6>; [INV]
> else
> goto <bb 5>; [INV]
>
> <bb 5> :
> __builtin_abort ();
>
> <bb 6> :
> b.2_4 ={v} b;
> _5 = b.2_4 != 0;
> _6 = (int) _5;
> return 972195717;
>
>
> Which has removed the second call to builtin_abort() (Even before we
> get to EVRP!)
>
> SO the issue doesn't seem to be removing the divide by 0, it seems to be
> a pattern match for [0,1] that is triggering.
>
> I would argue the test case should not be testing for not removing the
> divide by 0... Because we can now fold c_14 to be 486097858, and I
> think that is a valid transformation? (assuming no non-call exceptions
> of course)
I think it's a valid transform, even with -fnon-call-exceptions when
-fdelete-dead-exceptions is enabled (like for Ada and C++).
You'd have to dig into history to tell why we added this testcase in that way.
Richard.
>
> Andrew
>
