[PATCH] D103816: [SimpleLoopUnswich] Fix a bug on ComputeUnswitchedCost with partial unswitch

[JinGu Kang via Phabricator via llvm-commits]

jaykang10 created this revision.
jaykang10 added reviewers: fhahn, jonpa, sanwou01.
Herald added a subscriber: hiraditya.
jaykang10 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

There was a bug from cost calculation for partially invariant unswitch.

On omnetpp of SPEC2017, more functions are inlined with new pass manager's inliner against legacy pass manager one.

It causes bigger unswitch cost and I have found my update of the cost calculation is wrong from it.

A test case is updated because it has multiple unswitch candidates and the different candidate is picked up.


https://reviews.llvm.org/D103816

Files:
  llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
  llvm/test/Transforms/SimpleLoopUnswitch/partial-unswitch.ll


Index: llvm/test/Transforms/SimpleLoopUnswitch/partial-unswitch.ll
===================================================================
--- llvm/test/Transforms/SimpleLoopUnswitch/partial-unswitch.ll
+++ llvm/test/Transforms/SimpleLoopUnswitch/partial-unswitch.ll
@@ -1108,11 +1108,11 @@
 ; CHECK-LABEL: @partial_unswitch_exiting_block_with_multiple_unswitch_candidates(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[EXIT_COND:%.*]] = icmp ne i32 [[TMP0:%.*]], 0
-; CHECK-NEXT:    br i1 [[EXIT_COND]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
-; CHECK:       entry.split.us:
 ; CHECK-NEXT:    [[TMP2:%.*]] = load i32, i32* [[PTR:%.*]], align 16
 ; CHECK-NEXT:    [[TMP3:%.*]] = icmp ult i32 [[TMP2]], 41
-; CHECK-NEXT:    br i1 [[TMP3]], label [[ENTRY_SPLIT_US_SPLIT:%.*]], label [[ENTRY_SPLIT_US_SPLIT_US:%.*]]
+; CHECK-NEXT:    br i1 [[TMP3]], label [[ENTRY_SPLIT:%.*]], label [[ENTRY_SPLIT_US:%.*]]
+; CHECK:       entry.split.us:
+; CHECK-NEXT:    br i1 [[EXIT_COND]], label [[ENTRY_SPLIT_US_SPLIT_US:%.*]], label [[ENTRY_SPLIT_US_SPLIT:%.*]]
 ; CHECK:       entry.split.us.split.us:
 ; CHECK-NEXT:    br label [[LOOP_US_US:%.*]]
 ; CHECK:       loop.us.us:
@@ -1122,14 +1122,12 @@
 ; CHECK:       entry.split.us.split:
 ; CHECK-NEXT:    br label [[LOOP_US:%.*]]
 ; CHECK:       loop.us:
-; CHECK-NEXT:    [[VAL_US:%.*]] = load i32, i32* [[PTR]], align 16
-; CHECK-NEXT:    [[IF_COND_US:%.*]] = icmp ult i32 [[VAL_US]], 41
-; CHECK-NEXT:    br i1 [[IF_COND_US]], label [[IF_THEN_US:%.*]], label [[EXITING_US:%.*]]
-; CHECK:       if.then.us:
-; CHECK-NEXT:    store i32 [[TMP1:%.*]], i32* [[PTR]], align 16
-; CHECK-NEXT:    br label [[EXITING_US]]
+; CHECK-NEXT:    br label [[EXITING_US:%.*]]
 ; CHECK:       exiting.us:
-; CHECK-NEXT:    br label [[LOOP_US]], !llvm.loop [[LOOP10:![0-9]+]]
+; CHECK-NEXT:    br label [[EXIT_SPLIT_US:%.*]]
+; CHECK:       exit.split.us:
+; CHECK-NEXT:    [[RET_VAL_US:%.*]] = phi i32 [ 1, [[EXITING_US]] ]
+; CHECK-NEXT:    br label [[EXIT:%.*]]
 ; CHECK:       entry.split:
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
@@ -1137,13 +1135,16 @@
 ; CHECK-NEXT:    [[IF_COND:%.*]] = icmp ult i32 [[VAL]], 41
 ; CHECK-NEXT:    br i1 [[IF_COND]], label [[IF_THEN:%.*]], label [[EXITING:%.*]]
 ; CHECK:       if.then:
-; CHECK-NEXT:    store i32 [[TMP1]], i32* [[PTR]], align 16
+; CHECK-NEXT:    store i32 [[TMP1:%.*]], i32* [[PTR]], align 16
 ; CHECK-NEXT:    br label [[EXITING]]
 ; CHECK:       exiting:
-; CHECK-NEXT:    br label [[EXIT:%.*]]
-; CHECK:       exit:
+; CHECK-NEXT:    br i1 [[EXIT_COND]], label [[LOOP]], label [[EXIT_SPLIT:%.*]], !llvm.loop [[LOOP10:![0-9]+]]
+; CHECK:       exit.split:
 ; CHECK-NEXT:    [[RET_VAL:%.*]] = phi i32 [ 1, [[EXITING]] ]
-; CHECK-NEXT:    ret i32 [[RET_VAL]]
+; CHECK-NEXT:    br label [[EXIT]]
+; CHECK:       exit:
+; CHECK-NEXT:    [[DOTUS_PHI:%.*]] = phi i32 [ [[RET_VAL]], [[EXIT_SPLIT]] ], [ [[RET_VAL_US]], [[EXIT_SPLIT_US]] ]
+; CHECK-NEXT:    ret i32 [[DOTUS_PHI]]
 ;
 entry:
   %exit.cond = icmp ne i32 %0, 0
Index: llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
+++ llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
@@ -2868,12 +2868,12 @@
         } else if (match(BI.getCondition(), m_LogicalOr())) {
           if (SuccBB == BI.getSuccessor(0))
             continue;
-        } else if (!PartialIVInfo.InstToDuplicate.empty()) {
+        } else if (PartialIVCondBranch == &TI) {
           if (PartialIVInfo.KnownValue->isOneValue() &&
-              SuccBB == BI.getSuccessor(1))
+              SuccBB == BI.getSuccessor(0))
             continue;
           else if (!PartialIVInfo.KnownValue->isOneValue() &&
-                   SuccBB == BI.getSuccessor(0))
+                   SuccBB == BI.getSuccessor(1))
             continue;
         }
       }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D103816.350291.patch
Type: text/x-patch
Size: 3908 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210607/c5de8f52/attachment.bin>