On 7/29/21 3:19 AM, Richard Biener wrote:
On Wed, Jul 28, 2021 at 4:39 PM Andrew MacLeod <amacl...@redhat.com> wrote:
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.
Looks like the PR was adding an optimization to simplification which
checked to see if the LHS of a binop is a constant and the RHS of the
binop was a 2-value range.
If so, it tries folding each range individually and if that is
successful, turns it into a CONDITION ? LHSopRHS1 : LHSopRHS2
THere are 4 test files, the second one is triggering the failure because
it deals with / and %, and is specifically checking to make sure the
invalid cases like / 0 and % 0 are not transformed..
With EVRP now able to do something with both the const and the modulus,
the operations it is specifically scanning for to make sure it didn't
transform no longer apply.
In fact, the things it checks for:
/* Dont optimize 972195717 / 0 in function foo. */
/* { dg-final { scan-tree-dump-times "972195717 / " 1 "evrp" } } */
/* Dont optimize 972195717 % 0 in function bar. */
/* { dg-final { scan-tree-dump-times "972195717 % " 1 "evrp" } } */
/* May optimize in function bar2, but EVRP doesn't perform this yet. */
/* { dg-final { scan-tree-dump-times "972195715 % " 0 "evrp" } } */
The last line was already adjusted once when EVRP got smarter and
started doing the transformation on CST % 1 : 2
I see a couple of options... we could delete the testcase, but Im not
sure that is useful.
1) just change those 1's to 0's and move on like was time last time it
appear.
2) also add a check that __builtin_abort is only called once. This would
then be checking that we do all these transformations:
CST / [0,2] will produce CST/2
CST % [0, 2] will produce either [0,0] or [1,1] based on CST.
CST % [1,2] will produce [0,1], but be transformed from the % into the
?: form.
Is this acceptable? Test tetscase change would look like:
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr61839_2.c
b/gcc/testsuite/gcc.dg/tree-ssa/pr61839_2.c
index cfec54de991..e579fcb680f 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr61839_2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr61839_2.c
@@ -45,10 +45,14 @@ int bar2 ()
return 0;
}
+/* EVRP now transformations all 3 functions. */
+/* { dg-final { scan-tree-dump-times "__builtin_abort" 1 "evrp" } } */
/* Dont optimize 972195717 / 0 in function foo. */
-/* { dg-final { scan-tree-dump-times "972195717 / " 1 "evrp" } } */
+/* { dg-final { scan-tree-dump-times "972195717 / " 0 "evrp" } } */
/* Dont optimize 972195717 % 0 in function bar. */
-/* { dg-final { scan-tree-dump-times "972195717 % " 1 "evrp" } } */
+/* { dg-final { scan-tree-dump-times "972195717 % " 0 "evrp" } } */
/* May optimize in function bar2, but EVRP doesn't perform this yet. */
/* { dg-final { scan-tree-dump-times "972195715 % " 0 "evrp" } } */
+
+
