[PATCH] D39589: [IRCE] Remove folding of two range checks into RANGE_CHECK_BOTH

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev created this revision.

The logic of replacing of a couple `RANGE_CHECK_LOWER + RANGE_CHECK_UPPER`
into `RANGE_CHECK_BOTH` in fact duplicates the logic of range intersection which
happens when we calculate safe iteration space. Effectively, the result of intersection of
these ranges doesn't differ from the range of merged range check.

We chose to remove duplicating logic in favor of code simplicity.


https://reviews.llvm.org/D39589

Files:
  lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
  test/Transforms/IRCE/single-access-with-preloop.ll


Index: test/Transforms/IRCE/single-access-with-preloop.ll
===================================================================
--- test/Transforms/IRCE/single-access-with-preloop.ll
+++ test/Transforms/IRCE/single-access-with-preloop.ll
@@ -40,13 +40,24 @@
 
 
 ; CHECK: [[not_safe_start_2:[^ ]+]] = add i32 %offset, -1
-; CHECK: [[not_safe_end:[^ ]+]] = sub i32 [[not_safe_start_2]], %len
-; CHECK: [[not_exit_mainloop_at_cond_loclamp:[^ ]+]] = icmp sgt i32 [[not_safe_end]], [[not_n]]
-; CHECK: [[not_exit_mainloop_at_loclamp:[^ ]+]] = select i1 [[not_exit_mainloop_at_cond_loclamp]], i32 [[not_safe_end]], i32 [[not_n]]
-; CHECK: [[exit_mainloop_at_loclamp:[^ ]+]] = sub i32 -1, [[not_exit_mainloop_at_loclamp]]
-; CHECK: [[exit_mainloop_at_cmp:[^ ]+]] = icmp sgt i32 [[exit_mainloop_at_loclamp]], 0
-; CHECK: [[exit_mainloop_at:[^ ]+]] = select i1 [[exit_mainloop_at_cmp]], i32 [[exit_mainloop_at_loclamp]], i32 0
+; CHECK: [[not_safe_upper_end:[^ ]+]] = sub i32 [[not_safe_start_2]], %len
+; CHECK: [[not_exit_mainloop_at_cond_loclamp:[^ ]+]] = icmp sgt i32 [[not_safe_upper_end]], [[not_n]]
+; CHECK: [[not_exit_mainloop_at_loclamp:[^ ]+]] = select i1 [[not_exit_mainloop_at_cond_loclamp]], i32 [[not_safe_upper_end]], i32 [[not_n]]
+; CHECK: [[not_safe_lower_end:[^ ]+]] = add i32 %offset, -2147483648
+; CHECK: [[not_exit_mainloop_at_cond_hiclamp:[^ ]+]] = icmp sgt i32 [[not_exit_mainloop_at_loclamp]], [[not_safe_lower_end]]
+; CHECK: [[not_exit_mainloop_at_hiclamp:[^ ]+]] = select i1 [[not_exit_mainloop_at_cond_hiclamp]], i32 [[not_exit_mainloop_at_loclamp]], i32 [[not_safe_lower_end]]
+; CHECK: [[exit_mainloop_at_hiclamp:[^ ]+]] = sub i32 -1, [[not_exit_mainloop_at_hiclamp]]
+; CHECK: [[exit_mainloop_at_cmp:[^ ]+]] = icmp sgt i32 [[exit_mainloop_at_hiclamp]], 0
+; CHECK: [[exit_mainloop_at:[^ ]+]] = select i1 [[exit_mainloop_at_cmp]], i32 [[exit_mainloop_at_hiclamp]], i32 0
 
+; CHECK: mainloop:
+; CHECK: br label %loop
+
+; CHECK: loop:
+; CHECK: %abc.high = icmp slt i32 %array.idx, %len
+; CHECK: %abc.low = icmp sge i32 %array.idx, 0
+; CHECK: %abc = and i1 true, true
+; CHECK: br i1 %abc, label %in.bounds, label %out.of.bounds.loopexit8
 
 ; CHECK: in.bounds:
 ; CHECK: [[continue_mainloop_cond:[^ ]+]] = icmp slt i32 %idx.next, [[exit_mainloop_at]]
Index: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
===================================================================
--- lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
+++ lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
@@ -367,37 +367,12 @@
   if (!Visited.insert(Condition).second)
     return;
 
+  // TODO: Do the same for OR, XOR, NOT etc?
   if (match(Condition, m_And(m_Value(), m_Value()))) {
-    SmallVector<InductiveRangeCheck, 8> SubChecks;
     extractRangeChecksFromCond(L, SE, cast<User>(Condition)->getOperandUse(0),
-                               SubChecks, Visited);
+                               Checks, Visited);
     extractRangeChecksFromCond(L, SE, cast<User>(Condition)->getOperandUse(1),
-                               SubChecks, Visited);
-
-    if (SubChecks.size() == 2) {
-      // Handle a special case where we know how to merge two checks separately
-      // checking the upper and lower bounds into a full range check.
-      const auto &RChkA = SubChecks[0];
-      const auto &RChkB = SubChecks[1];
-      if ((RChkA.End == RChkB.End || !RChkA.End || !RChkB.End) &&
-          RChkA.Begin == RChkB.Begin && RChkA.Step == RChkB.Step &&
-          RChkA.IsSigned == RChkB.IsSigned) {
-        // If RChkA.Kind == RChkB.Kind then we just found two identical checks.
-        // But if one of them is a RANGE_CHECK_LOWER and the other is a
-        // RANGE_CHECK_UPPER (only possibility if they're different) then
-        // together they form a RANGE_CHECK_BOTH.
-        SubChecks[0].Kind =
-            (InductiveRangeCheck::RangeCheckKind)(RChkA.Kind | RChkB.Kind);
-        SubChecks[0].End = RChkA.End ? RChkA.End : RChkB.End;
-        SubChecks[0].CheckUse = &ConditionUse;
-        SubChecks[0].IsSigned = RChkA.IsSigned;
-
-        // We updated one of the checks in place, now erase the other.
-        SubChecks.pop_back();
-      }
-    }
-
-    Checks.insert(Checks.end(), SubChecks.begin(), SubChecks.end());
+                               Checks, Visited);
     return;
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D39589.121445.patch
Type: text/x-patch
Size: 4343 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171103/720e8561/attachment.bin>