[llvm] 0c4bbd2 - [IRSim] Make sure the first instruction of a block doesn't get missed if it is the first valid instruction in Module.

Andrew Litteken via llvm-commits llvm-commits at lists.llvm.org
Sun Mar 13 21:19:08 PDT 2022


Author: Andrew Litteken
Date: 2022-03-13T23:13:09-05:00
New Revision: 0c4bbd293e6676cf7c45b95ad0890e0c1e423c3d

URL: https://github.com/llvm/llvm-project/commit/0c4bbd293e6676cf7c45b95ad0890e0c1e423c3d
DIFF: https://github.com/llvm/llvm-project/commit/0c4bbd293e6676cf7c45b95ad0890e0c1e423c3d.diff

LOG: [IRSim] Make sure the first instruction of a block doesn't get missed if it is the first valid instruction in Module.

If an instruction is first legal instruction in the module, and is the only legal instruction in its basic block, it will be ignored by the outliner due to a length check inherited from the older version of the outliner that was restricted to outlining within a single basic block. This removes that check, and updates any tests that broke because of it.

Reviewer: paquette

Differential Revision: https://reviews.llvm.org/D120786

Added: 
    llvm/test/Transforms/IROutliner/outlining-first-instruction.ll

Modified: 
    llvm/lib/Analysis/IRSimilarityIdentifier.cpp
    llvm/test/Transforms/IROutliner/outlining-basic-branches.ll
    llvm/test/Transforms/IROutliner/outlining-no-return-functions.ll
    llvm/test/Transforms/IROutliner/phi-nodes-simple.ll
    llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/IRSimilarityIdentifier.cpp b/llvm/lib/Analysis/IRSimilarityIdentifier.cpp
index fea50c43d85bb..468edc2b92d00 100644
--- a/llvm/lib/Analysis/IRSimilarityIdentifier.cpp
+++ b/llvm/lib/Analysis/IRSimilarityIdentifier.cpp
@@ -289,14 +289,12 @@ void IRInstructionMapper::convertToUnsignedVec(
     }
   }
 
-  if (HaveLegalRange) {
-    if (AddedIllegalLastTime)
-      mapToIllegalUnsigned(It, IntegerMappingForBB, InstrListForBB, true);
-    for (IRInstructionData *ID : InstrListForBB)
-      this->IDL->push_back(*ID);
-    llvm::append_range(InstrList, InstrListForBB);
-    llvm::append_range(IntegerMapping, IntegerMappingForBB);
-  }
+  if (AddedIllegalLastTime)
+    mapToIllegalUnsigned(It, IntegerMappingForBB, InstrListForBB, true);
+  for (IRInstructionData *ID : InstrListForBB)
+    this->IDL->push_back(*ID);
+  llvm::append_range(InstrList, InstrListForBB);
+  llvm::append_range(IntegerMapping, IntegerMappingForBB);
 }
 
 // TODO: This is the same as the MachineOutliner, and should be consolidated

diff  --git a/llvm/test/Transforms/IROutliner/outlining-basic-branches.ll b/llvm/test/Transforms/IROutliner/outlining-basic-branches.ll
index b00eec1155114..955036cc8bfaf 100644
--- a/llvm/test/Transforms/IROutliner/outlining-basic-branches.ll
+++ b/llvm/test/Transforms/IROutliner/outlining-basic-branches.ll
@@ -10,12 +10,12 @@ entry:
 next:
   br label %next2
 next2:
-  br label %next
+  br label %next3
 next3:
   %a = alloca i32, align 4
   br label %next4
 next4:
-  br label %next3
+  br label %next5
 next5:
   br label %next6
 next6:
@@ -25,15 +25,11 @@ next6:
 
 ; CHECK-LABEL: @outline_outputs1(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    br label [[NEXT:%.*]]
-; CHECK:       next:
 ; CHECK-NEXT:    call void @outlined_ir_func_0()
-; CHECK-NEXT:    br label [[NEXT]]
+; CHECK-NEXT:    br label [[NEXT3:%.*]]
 ; CHECK:       next3:
 ; CHECK-NEXT:    [[A:%.*]] = alloca i32, align 4
 ; CHECK-NEXT:    call void @outlined_ir_func_0()
-; CHECK-NEXT:    br label [[NEXT3:%.*]]
-; CHECK:       next5:
 ; CHECK-NEXT:    br label [[NEXT6:%.*]]
 ; CHECK:       next6:
 ; CHECK-NEXT:    [[B:%.*]] = alloca i32, align 4
@@ -41,12 +37,14 @@ next6:
 ;
 ;
 ; CHECK: define internal void @outlined_ir_func_0(
-; CHECK:  newFuncRoot:
-; CHECK-NEXT:    br label [[NEXT_TO_OUTLINE:%.*]]
-; CHECK:       next_to_outline:
+; CHECK-NEXT:  newFuncRoot:
+; CHECK-NEXT:    br label [[ENTRY_TO_OUTLINE:%.*]]
+; CHECK:       entry_to_outline:
+; CHECK-NEXT:    br label [[NEXT:%.*]]
+; CHECK:       next:
 ; CHECK-NEXT:    br label [[NEXT2:%.*]]
 ; CHECK:       next2:
-; CHECK-NEXT:    br label [[NEXT_EXITSTUB:%.*]]
-; CHECK:       next.exitStub:
+; CHECK-NEXT:    br label [[NEXT3_EXITSTUB:%.*]]
+; CHECK:       next3.exitStub:
 ; CHECK-NEXT:    ret void
 ;

diff  --git a/llvm/test/Transforms/IROutliner/outlining-first-instruction.ll b/llvm/test/Transforms/IROutliner/outlining-first-instruction.ll
new file mode 100644
index 0000000000000..7e2ec19054483
--- /dev/null
+++ b/llvm/test/Transforms/IROutliner/outlining-first-instruction.ll
@@ -0,0 +1,67 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --include-generated-funcs
+; RUN: opt -S -verify -iroutliner -ir-outlining-no-cost < %s | FileCheck %s
+
+; Make sure that we outline from all three of these functions, and that
+; the first instruction in the module is included when it is the only
+; instruction in the first basic block.
+
+define void @f1() {
+bb:
+  br label %bb1
+bb1:
+  br label %bb2
+bb2:
+  ret void
+}
+
+define void @f2() {
+bb:
+  br label %bb1
+bb1:
+  br label %bb2
+bb2:
+  ret void
+}
+
+define void @f3() {
+bb:
+  br label %bb1
+bb1:
+  br label %bb2
+bb2:
+  ret void
+}
+; CHECK-LABEL: @f1(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    call void @outlined_ir_func_0()
+; CHECK-NEXT:    br label [[BB2:%.*]]
+; CHECK:       bb2:
+; CHECK-NEXT:    ret void
+;
+;
+; CHECK-LABEL: @f2(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    call void @outlined_ir_func_0()
+; CHECK-NEXT:    br label [[BB2:%.*]]
+; CHECK:       bb2:
+; CHECK-NEXT:    ret void
+;
+;
+; CHECK-LABEL: @f3(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    call void @outlined_ir_func_0()
+; CHECK-NEXT:    br label [[BB2:%.*]]
+; CHECK:       bb2:
+; CHECK-NEXT:    ret void
+;
+;
+; CHECK-LABEL: define internal void @outlined_ir_func_0(
+; CHECK-NEXT:  newFuncRoot:
+; CHECK-NEXT:    br label [[BB_TO_OUTLINE:%.*]]
+; CHECK:       bb_to_outline:
+; CHECK-NEXT:    br label [[BB1:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    br label [[BB2_EXITSTUB:%.*]]
+; CHECK:       bb2.exitStub:
+; CHECK-NEXT:    ret void
+;

diff  --git a/llvm/test/Transforms/IROutliner/outlining-no-return-functions.ll b/llvm/test/Transforms/IROutliner/outlining-no-return-functions.ll
index 1a7af015c0f81..3d365331ca43e 100644
--- a/llvm/test/Transforms/IROutliner/outlining-no-return-functions.ll
+++ b/llvm/test/Transforms/IROutliner/outlining-no-return-functions.ll
@@ -28,9 +28,8 @@ bb1:
 }
 ; CHECK-LABEL: @f1(
 ; CHECK-NEXT:  bb:
-; CHECK-NEXT:    br label [[BB1:%.*]]
-; CHECK:       bb1:
-; CHECK-NEXT:    br label [[BB1]]
+; CHECK-NEXT:    call void @outlined_ir_func_0()
+; CHECK-NEXT:    ret void
 ;
 ;
 ; CHECK-LABEL: @f2(

diff  --git a/llvm/test/Transforms/IROutliner/phi-nodes-simple.ll b/llvm/test/Transforms/IROutliner/phi-nodes-simple.ll
index e5afb89a26138..6b270073e1302 100644
--- a/llvm/test/Transforms/IROutliner/phi-nodes-simple.ll
+++ b/llvm/test/Transforms/IROutliner/phi-nodes-simple.ll
@@ -29,30 +29,28 @@ first:
 }
 ; CHECK-LABEL: @function1(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    br label [[TEST:%.*]]
-; CHECK:       test:
 ; CHECK-NEXT:    call void @outlined_ir_func_0(i32* [[A:%.*]], i32* [[B:%.*]])
 ; CHECK-NEXT:    ret void
 ;
 ;
 ; CHECK-LABEL: @function2(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    br label [[TEST:%.*]]
-; CHECK:       test:
 ; CHECK-NEXT:    call void @outlined_ir_func_0(i32* [[A:%.*]], i32* [[B:%.*]])
 ; CHECK-NEXT:    ret void
 ;
 ;
 ; CHECK: define internal void @outlined_ir_func_0(
 ; CHECK-NEXT:  newFuncRoot:
-; CHECK-NEXT:    br label [[TEST_TO_OUTLINE:%.*]]
-; CHECK:       test_to_outline:
+; CHECK-NEXT:    br label [[ENTRY_TO_OUTLINE:%.*]]
+; CHECK:       entry_to_outline:
+; CHECK-NEXT:    br label [[TEST:%.*]]
+; CHECK:       test:
 ; CHECK-NEXT:    br label [[FIRST:%.*]]
 ; CHECK:       first:
-; CHECK-NEXT:    [[TMP2:%.*]] = phi i32 [ 0, [[TEST_TO_OUTLINE]] ]
+; CHECK-NEXT:    [[TMP2:%.*]] = phi i32 [ 0, [[TEST]] ]
 ; CHECK-NEXT:    store i32 2, i32* [[TMP0:%.*]], align 4
 ; CHECK-NEXT:    store i32 3, i32* [[TMP1:%.*]], align 4
-; CHECK-NEXT:    br label [[TEST_AFTER_OUTLINE_EXITSTUB:%.*]]
-; CHECK:       test_after_outline.exitStub:
+; CHECK-NEXT:    br label [[ENTRY_AFTER_OUTLINE_EXITSTUB:%.*]]
+; CHECK:       entry_after_outline.exitStub:
 ; CHECK-NEXT:    ret void
 ;

diff  --git a/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp b/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp
index 3b8bd74354f2f..00fa937d8ad9d 100644
--- a/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp
+++ b/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp
@@ -809,7 +809,8 @@ TEST(IRInstructionMapper, PhiIllegal) {
   getVectors(*M, Mapper, InstrList, UnsignedVec);
 
   ASSERT_EQ(InstrList.size(), UnsignedVec.size());
-  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(0));
+  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(3));
+  ASSERT_GT(UnsignedVec[0], Mapper.IllegalInstrNumber);
 }
 
 // In most cases, the illegal instructions we are collecting don't require any
@@ -844,7 +845,8 @@ TEST(IRInstructionMapper, AllocaIllegal) {
   getVectors(*M, Mapper, InstrList, UnsignedVec);
 
   ASSERT_EQ(InstrList.size(), UnsignedVec.size());
-  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(0));
+  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(1));
+  ASSERT_GT(UnsignedVec[0], Mapper.IllegalInstrNumber);
 }
 
 // Checks that an getelementptr instruction is mapped to be legal.  And that
@@ -983,7 +985,8 @@ TEST(IRInstructionMapper, CallsIllegalIndirect) {
   getVectors(*M, Mapper, InstrList, UnsignedVec);
 
   ASSERT_EQ(InstrList.size(), UnsignedVec.size());
-  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(0));
+  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(1));
+  ASSERT_GT(UnsignedVec[0], Mapper.IllegalInstrNumber);
 }
 
 // Checks that indirect call instructions are mapped to be legal when it is not
@@ -1265,7 +1268,8 @@ TEST(IRInstructionMapper, InvokeIllegal) {
   getVectors(*M, Mapper, InstrList, UnsignedVec);
 
   ASSERT_EQ(InstrList.size(), UnsignedVec.size());
-  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(0));
+  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(1));
+  ASSERT_GT(UnsignedVec[0], Mapper.IllegalInstrNumber);
 }
 
 // Checks that an callbr instructions are considered to be illegal.  Callbr
@@ -1298,7 +1302,8 @@ TEST(IRInstructionMapper, CallBrInstIllegal) {
   getVectors(*M, Mapper, InstrList, UnsignedVec);
 
   ASSERT_EQ(InstrList.size(), UnsignedVec.size());
-  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(0));
+  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(1));
+  ASSERT_GT(UnsignedVec[0], Mapper.IllegalInstrNumber);
 }
 
 // Checks that an debuginfo intrinsics are mapped to be invisible.  Since they
@@ -1356,7 +1361,8 @@ TEST(IRInstructionMapper, ExceptionHandlingTypeIdIllegal) {
   getVectors(*M, Mapper, InstrList, UnsignedVec);
 
   ASSERT_EQ(InstrList.size(), UnsignedVec.size());
-  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(0));
+  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(1));
+  ASSERT_GT(UnsignedVec[0], Mapper.IllegalInstrNumber);
 }
 
 // Checks that an eh.exceptioncode intrinsic is mapped to be illegal.
@@ -1388,7 +1394,8 @@ TEST(IRInstructionMapper, ExceptionHandlingExceptionCodeIllegal) {
   getVectors(*M, Mapper, InstrList, UnsignedVec);
 
   ASSERT_EQ(InstrList.size(), UnsignedVec.size());
-  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(0));
+  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(1));
+  ASSERT_GT(UnsignedVec[0], Mapper.IllegalInstrNumber);
 }
 
 // Checks that an eh.unwind intrinsic is mapped to be illegal.
@@ -1413,7 +1420,8 @@ TEST(IRInstructionMapper, ExceptionHandlingUnwindIllegal) {
   getVectors(*M, Mapper, InstrList, UnsignedVec);
 
   ASSERT_EQ(InstrList.size(), UnsignedVec.size());
-  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(0));
+  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(1));
+  ASSERT_GT(UnsignedVec[0], Mapper.IllegalInstrNumber);
 }
 
 // Checks that an eh.exceptionpointer intrinsic is mapped to be illegal.
@@ -1438,7 +1446,8 @@ TEST(IRInstructionMapper, ExceptionHandlingExceptionPointerIllegal) {
   getVectors(*M, Mapper, InstrList, UnsignedVec);
 
   ASSERT_EQ(InstrList.size(), UnsignedVec.size());
-  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(0));
+  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(1));
+  ASSERT_GT(UnsignedVec[0], Mapper.IllegalInstrNumber);
 }
 
 // Checks that a catchpad instruction is mapped to an illegal value.
@@ -1469,7 +1478,8 @@ TEST(IRInstructionMapper, CatchpadIllegal) {
   getVectors(*M, Mapper, InstrList, UnsignedVec);
 
   ASSERT_EQ(InstrList.size(), UnsignedVec.size());
-  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(0));
+  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(1));
+  ASSERT_GT(UnsignedVec[0], Mapper.IllegalInstrNumber);
 }
 
 // Checks that a cleanuppad instruction is mapped to an illegal value.
@@ -1500,7 +1510,8 @@ TEST(IRInstructionMapper, CleanuppadIllegal) {
   getVectors(*M, Mapper, InstrList, UnsignedVec);
 
   ASSERT_EQ(InstrList.size(), UnsignedVec.size());
-  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(0));
+  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(1));
+  ASSERT_GT(UnsignedVec[0], Mapper.IllegalInstrNumber);
 }
 
 // The following three instructions are memory transfer and setting based, which
@@ -2462,7 +2473,7 @@ TEST(IRSimilarityCandidate, SamePHIStructureInternal) {
   getVectors(*M, Mapper, InstrList, UnsignedVec);
 
   // Check to make sure that we have a long enough region.
-  ASSERT_EQ(InstrList.size(), static_cast<unsigned>(11));
+  ASSERT_EQ(InstrList.size(), static_cast<unsigned>(12));
   // Check that the instructions were added correctly to both vectors.
   ASSERT_TRUE(InstrList.size() == UnsignedVec.size());
 
@@ -2515,7 +2526,7 @@ TEST(IRSimilarityCandidate, DifferentPHIStructureInternal) {
   getVectors(*M, Mapper, InstrList, UnsignedVec);
 
   // Check to make sure that we have a long enough region.
-  ASSERT_EQ(InstrList.size(), static_cast<unsigned>(13));
+  ASSERT_EQ(InstrList.size(), static_cast<unsigned>(14));
   // Check that the instructions were added correctly to both vectors.
   ASSERT_TRUE(InstrList.size() == UnsignedVec.size());
 


        


More information about the llvm-commits mailing list