[llvm] r371707 - Make SwitchInstProfUpdateWrapper strict permanently
Yevgeny Rouban via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 11 20:41:34 PDT 2019
Author: yrouban
Date: Wed Sep 11 20:41:34 2019
New Revision: 371707
URL: http://llvm.org/viewvc/llvm-project?rev=371707&view=rev
Log:
Make SwitchInstProfUpdateWrapper strict permanently
We have been using -switch-inst-prof-update-wrapper-strict
set to true by default for some time. It is time to remove
the safety stuff and make SwitchInstProfUpdateWrapper
intolerant to inconsistencies in !prof branch_weights
metadata of SwitchInst.
This patch gets rid of the Invalid state of
SwitchInstProfUpdateWrapper and the option
-switch-inst-prof-update-wrapper-strict. So there is only
two states: changed and unchanged.
Reviewers: davidx, nikic, eraman, reames, chandlerc
Reviewed By: davidx
Differential Revision: https://reviews.llvm.org/D67435
Modified:
llvm/trunk/include/llvm/IR/Instructions.h
llvm/trunk/lib/IR/Instructions.cpp
Modified: llvm/trunk/include/llvm/IR/Instructions.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Instructions.h?rev=371707&r1=371706&r2=371707&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/Instructions.h (original)
+++ llvm/trunk/include/llvm/IR/Instructions.h Wed Sep 11 20:41:34 2019
@@ -3459,16 +3459,7 @@ public:
class SwitchInstProfUpdateWrapper {
SwitchInst &SI;
Optional<SmallVector<uint32_t, 8> > Weights = None;
-
- // Sticky invalid state is needed to safely ignore operations with prof data
- // in cases where SwitchInstProfUpdateWrapper is created from SwitchInst
- // with inconsistent prof data. TODO: once we fix all prof data
- // inconsistencies we can turn invalid state to assertions.
- enum {
- Invalid,
- Initialized,
- Changed
- } State = Invalid;
+ bool Changed = false;
protected:
static MDNode *getProfBranchWeightsMD(const SwitchInst &SI);
@@ -3486,7 +3477,7 @@ public:
SwitchInstProfUpdateWrapper(SwitchInst &SI) : SI(SI) { init(); }
~SwitchInstProfUpdateWrapper() {
- if (State == Changed)
+ if (Changed)
SI.setMetadata(LLVMContext::MD_prof, buildProfBranchWeightsMD());
}
Modified: llvm/trunk/lib/IR/Instructions.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Instructions.cpp?rev=371707&r1=371706&r2=371707&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Instructions.cpp (original)
+++ llvm/trunk/lib/IR/Instructions.cpp Wed Sep 11 20:41:34 2019
@@ -45,12 +45,6 @@
using namespace llvm;
-static cl::opt<bool> SwitchInstProfUpdateWrapperStrict(
- "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(true));
-
//===----------------------------------------------------------------------===//
// AllocaInst Class
//===----------------------------------------------------------------------===//
@@ -3897,7 +3891,7 @@ SwitchInstProfUpdateWrapper::getProfBran
}
MDNode *SwitchInstProfUpdateWrapper::buildProfBranchWeightsMD() {
- assert(State == Changed && "called only if metadata has changed");
+ assert(Changed && "called only if metadata has changed");
if (!Weights)
return nullptr;
@@ -3916,17 +3910,12 @@ MDNode *SwitchInstProfUpdateWrapper::bui
void SwitchInstProfUpdateWrapper::init() {
MDNode *ProfileData = getProfBranchWeightsMD(SI);
- if (!ProfileData) {
- State = Initialized;
+ if (!ProfileData)
return;
- }
if (ProfileData->getNumOperands() != SI.getNumSuccessors() + 1) {
- State = Invalid;
- if (SwitchInstProfUpdateWrapperStrict)
- llvm_unreachable("number of prof branch_weights metadata operands does "
- "not correspond to number of succesors");
- return;
+ llvm_unreachable("number of prof branch_weights metadata operands does "
+ "not correspond to number of succesors");
}
SmallVector<uint32_t, 8> Weights;
@@ -3935,7 +3924,6 @@ void SwitchInstProfUpdateWrapper::init()
uint32_t CW = C->getValue().getZExtValue();
Weights.push_back(CW);
}
- State = Initialized;
this->Weights = std::move(Weights);
}
@@ -3944,7 +3932,7 @@ SwitchInstProfUpdateWrapper::removeCase(
if (Weights) {
assert(SI.getNumSuccessors() == Weights->size() &&
"num of prof branch_weights must accord with num of successors");
- State = Changed;
+ Changed = true;
// Copy the last case to the place of the removed one and shrink.
// This is tightly coupled with the way SwitchInst::removeCase() removes
// the cases in SwitchInst::removeCase(CaseIt).
@@ -3959,15 +3947,12 @@ void SwitchInstProfUpdateWrapper::addCas
SwitchInstProfUpdateWrapper::CaseWeightOpt W) {
SI.addCase(OnVal, Dest);
- if (State == Invalid)
- return;
-
if (!Weights && W && *W) {
- State = Changed;
+ Changed = true;
Weights = SmallVector<uint32_t, 8>(SI.getNumSuccessors(), 0);
Weights.getValue()[SI.getNumSuccessors() - 1] = *W;
} else if (Weights) {
- State = Changed;
+ Changed = true;
Weights.getValue().push_back(W ? *W : 0);
}
if (Weights)
@@ -3978,11 +3963,9 @@ void SwitchInstProfUpdateWrapper::addCas
SymbolTableList<Instruction>::iterator
SwitchInstProfUpdateWrapper::eraseFromParent() {
// Instruction is erased. Mark as unchanged to not touch it in the destructor.
- if (State != Invalid) {
- State = Initialized;
- if (Weights)
- Weights->resize(0);
- }
+ Changed = false;
+ if (Weights)
+ Weights->resize(0);
return SI.eraseFromParent();
}
@@ -3995,7 +3978,7 @@ SwitchInstProfUpdateWrapper::getSuccesso
void SwitchInstProfUpdateWrapper::setSuccessorWeight(
unsigned idx, SwitchInstProfUpdateWrapper::CaseWeightOpt W) {
- if (!W || State == Invalid)
+ if (!W)
return;
if (!Weights && *W)
@@ -4004,7 +3987,7 @@ void SwitchInstProfUpdateWrapper::setSuc
if (Weights) {
auto &OldW = Weights.getValue()[idx];
if (*W != OldW) {
- State = Changed;
+ Changed = true;
OldW = *W;
}
}
More information about the llvm-commits
mailing list