[PATCH] D64061: Force check prof branch_weights consistency in SwitchInstProfUpdateWrapper

[Yevgeny Rouban via Phabricator via llvm-commits]

yrouban created this revision.
yrouban added reviewers: davidxl, nikic, reames, eraman, chandlerc.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.

This patch turns on the prof branch_weights metadata consistency check in //SwitchInstProfUpdateWrapper//.
Related part of the unit test is removed.
If this patch causes a failure then please before reverting do report the IR that hits the assertion and try identifying the pass that introduces the inconsistency. We have to fix all such passes.
See also the upcoming change D61179 <https://reviews.llvm.org/D61179> in the //Verifier//.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64061

Files:
  llvm/lib/IR/Instructions.cpp
  llvm/unittests/IR/InstructionsTest.cpp


Index: llvm/unittests/IR/InstructionsTest.cpp
===================================================================
--- llvm/unittests/IR/InstructionsTest.cpp
+++ llvm/unittests/IR/InstructionsTest.cpp
@@ -792,44 +792,6 @@
     EXPECT_EQ(*SIW.getSuccessorWeight(1), 11u);
     EXPECT_EQ(*SIW.getSuccessorWeight(2), 22u);
   }
-
-  // Make prof data invalid by adding one extra weight.
-  SI->setMetadata(LLVMContext::MD_prof, MDBuilder(C).createBranchWeights(
-                                            { 99, 11, 22, 33 })); // extra
-  { // Invalid prof data makes wrapper act as if there were no prof data.
-    SwitchInstProfUpdateWrapper SIW(*SI);
-    ASSERT_FALSE(SIW.getSuccessorWeight(0).hasValue());
-    ASSERT_FALSE(SIW.getSuccessorWeight(1).hasValue());
-    ASSERT_FALSE(SIW.getSuccessorWeight(2).hasValue());
-    SIW.addCase(ConstantInt::get(Int32Ty, 3), BB3.get(), 39);
-    ASSERT_FALSE(SIW.getSuccessorWeight(3).hasValue()); // did not add weight 39
-  }
-
-  { // With added 3rd case the prof data become consistent with num of cases.
-    SwitchInstProfUpdateWrapper SIW(*SI);
-    EXPECT_EQ(*SIW.getSuccessorWeight(0), 99u);
-    EXPECT_EQ(*SIW.getSuccessorWeight(1), 11u);
-    EXPECT_EQ(*SIW.getSuccessorWeight(2), 22u);
-    EXPECT_EQ(*SIW.getSuccessorWeight(3), 33u);
-  }
-
-  // Make prof data invalid by removing one extra weight.
-  SI->setMetadata(LLVMContext::MD_prof,
-                  MDBuilder(C).createBranchWeights({ 99, 11, 22 })); // shorter
-  { // Invalid prof data makes wrapper act as if there were no prof data.
-    SwitchInstProfUpdateWrapper SIW(*SI);
-    ASSERT_FALSE(SIW.getSuccessorWeight(0).hasValue());
-    ASSERT_FALSE(SIW.getSuccessorWeight(1).hasValue());
-    ASSERT_FALSE(SIW.getSuccessorWeight(2).hasValue());
-    SIW.removeCase(SwitchInst::CaseIt(SI, 2));
-  }
-
-  { // With removed 3rd case the prof data become consistent with num of cases.
-    SwitchInstProfUpdateWrapper SIW(*SI);
-    EXPECT_EQ(*SIW.getSuccessorWeight(0), 99u);
-    EXPECT_EQ(*SIW.getSuccessorWeight(1), 11u);
-    EXPECT_EQ(*SIW.getSuccessorWeight(2), 22u);
-  }
 }
 
 TEST(InstructionsTest, CommuteShuffleMask) {
Index: llvm/lib/IR/Instructions.cpp
===================================================================
--- llvm/lib/IR/Instructions.cpp
+++ llvm/lib/IR/Instructions.cpp
@@ -49,7 +49,7 @@
     "switch-inst-prof-update-wrapper-strict", cl::Hidden,
     cl::desc("Assert that prof branch_weights metadata is valid when creating "
              "an instance of SwitchInstProfUpdateWrapper"),
-    cl::init(false));
+    cl::init(true));
 
 //===----------------------------------------------------------------------===//
 //                            AllocaInst Class


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D64061.207479.patch
Type: text/x-patch
Size: 2718 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190702/7aaf59fc/attachment.bin>