[PATCH] D62656: Make SwitchInstProfUpdateWrapper safer

David Li via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 30 11:21:52 PDT 2019


davidxl added a comment.

What I meant is that we should never see inconsistent state and instead of setting invalid state, we assert there.   Ideally, the profile setting/update APIs should help user avoid creating the inconsistent state in the first place (i.e., catch the problem on the spot).



================
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
----------------
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.


================
Comment at: llvm/unittests/IR/InstructionsTest.cpp:813
+    EXPECT_EQ(*SIW.getSuccessorWeight(2), 22u);
+    EXPECT_EQ(*SIW.getSuccessorWeight(3), 33u);
+  }
----------------
why it is not 39?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62656/new/

https://reviews.llvm.org/D62656





More information about the llvm-commits mailing list