Remove prof branch weights if switch has 0-1 cases or no branch-weights set in SwitchInstProfUpdateWrapper #163120
Conversation
…ts set in SwitchInstProfUpdateWrapper If the switch statement does not have any cases, or has only one case, or if no weights were set through the ProfUpdate wrapper, such switch statements should not have prof branch weights metadata. And this is what we did prior to commit 240b73e.
|
@llvm/pr-subscribers-llvm-ir Author: Manish Kausik H (Nirhar) ChangesIf the switch statement does not have any cases, or has only one case, or if no weights were set through the ProfUpdate wrapper, such switch statements should not have prof branch weights metadata. And this is what we did prior to commit 240b73e. Full diff: https://github.com/llvm/llvm-project/pull/163120.diff 1 Files Affected:
diff --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h
index 27930bbc651bd..08f89ea00375c 100644
--- a/llvm/include/llvm/IR/Instructions.h
+++ b/llvm/include/llvm/IR/Instructions.h
@@ -3548,8 +3548,12 @@ class SwitchInstProfUpdateWrapper {
SwitchInstProfUpdateWrapper(SwitchInst &SI) : SI(SI) { init(); }
~SwitchInstProfUpdateWrapper() {
- if (Changed && Weights.has_value() && Weights->size() >= 2)
- setBranchWeights(SI, Weights.value(), /*IsExpected=*/false);
+ if (Changed) {
+ if (Weights.has_value() && Weights->size() >= 2)
+ setBranchWeights(SI, Weights.value(), /*IsExpected=*/false);
+ else
+ SI.setMetadata(LLVMContext::MD_prof, nullptr);
+ }
}
/// Delegate the call to the underlying SwitchInst::removeCase() and remove
|
|
cc @mtrofin |
|
ping @mtrofin! It would be great if you have a look, as this is an area that you touched recently. |
There was a problem hiding this comment.
What is the motivation?
Should it set the branch weights to "unknown" instead? Or the idea would be to catch the case where the weights get removed in profcheck?
Do any new tests fail under profcheck w your change?
See https://discourse.llvm.org/t/rfc-profile-information-propagation-unittesting/73595/11
3 participants
If the switch statement does not have any cases, or has only one case, or if no weights were set through the ProfUpdate wrapper, such switch statements should not have prof branch weights metadata. And this is what we did prior to commit 240b73e.