[PATCH] D62656: Make SwitchInstProfUpdateWrapper safer

Yevgeny Rouban via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 30 21:06:24 PDT 2019

yrouban marked 3 inline comments as done.
yrouban added inline comments.

Comment at: llvm/unittests/IR/InstructionsTest.cpp:797
+  // Make prof data invalid by adding one extra weight.
+  SI->setMetadata(LLVMContext::MD_prof,
+                  MDBuilder(C).createBranchWeights({ 99, 11, 22, 33})); // extra
davidxl wrote:
> looks like exposing the raw interface to setMetadata for SI is not safe. Using wrapper interface can help catch error that leads to inconsistent state.
it is just a test and SI is just a SwitchInst.

Comment at: llvm/unittests/IR/InstructionsTest.cpp:813
+    EXPECT_EQ(*SIW.getSuccessorWeight(2), 22u);
+    EXPECT_EQ(*SIW.getSuccessorWeight(3), 33u);
+  }
davidxl wrote:
> why it is not 39?
That is because 39 was set when SIW was inconsistent. So //SwitchInstProfUpdateWrapper::addCase()// skipped setting the weight 39 and delegated addCase() to the underlying SwitchInst. The prof data attached to the underlying SwicthInst was kept intact and had 4 operands: 99, 11, 22, 33. So the next time we created a new wrapper it started with valid state.



More information about the llvm-commits mailing list