Revert "[llvm][IR] Extend BranchWeightMetadata to track provenance of weights" (llvm#95060)

Reverts llvm#86609

This change causes compile-time regressions for stage2 builds
(https://llvm-compile-time-tracker.com/compare.php?from=3254f31a66263ea9647c9547f1531c3123444fcd&to=c5978f1eb5eeca8610b9dfce1fcbf1f473911cd8&stat=instructions:u).
It also introduced unintended changes to `.text` which should be
addressed before relanding.

Author: ilovepi

clang/test/CodeGenCXX/attr-likelihood-if-vs-builtin-expect.cpp

@@ -221,5 +221,5 @@ void tu2(int &i) {
   }
 }
 
-// CHECK: [[BW_LIKELY]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
-// CHECK: [[BW_UNLIKELY]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
+// CHECK: [[BW_LIKELY]] = !{!"branch_weights", i32 2000, i32 1}
+// CHECK: [[BW_UNLIKELY]] = !{!"branch_weights", i32 1, i32 2000}

llvm/docs/BranchWeightMetadata.rst

@@ -28,14 +28,11 @@ Supported Instructions
 
 Metadata is only assigned to the conditional branches. There are two extra
 operands for the true and the false branch.
-We optionally track if the metadata was added by ``__builtin_expect`` or
-``__builtin_expect_with_probability`` with an optional field ``!"expected"``.
 
 .. code-block:: none
 
   !0 = !{
     !"branch_weights",
-    [ !"expected", ]
     i32 <TRUE_BRANCH_WEIGHT>,
     i32 <FALSE_BRANCH_WEIGHT>
   }
@@ -50,7 +47,6 @@ is always case #0).
 
   !0 = !{
     !"branch_weights",
-    [ !"expected", ]
     i32 <DEFAULT_BRANCH_WEIGHT>
     [ , i32 <CASE_BRANCH_WEIGHT> ... ]
   }
@@ -64,7 +60,6 @@ Branch weights are assigned to every destination.
 
   !0 = !{
     !"branch_weights",
-    [ !"expected", ]
     i32 <LABEL_BRANCH_WEIGHT>
     [ , i32 <LABEL_BRANCH_WEIGHT> ... ]
   }
@@ -80,7 +75,6 @@ block and entry counts which may not be accurate with sampling.
 
   !0 = !{
     !"branch_weights",
-    [ !"expected", ]
     i32 <CALL_BRANCH_WEIGHT>
   }
 
@@ -101,7 +95,6 @@ is used.
 
   !0 = !{
     !"branch_weights",
-    [ !"expected", ]
     i32 <INVOKE_NORMAL_WEIGHT>
     [ , i32 <INVOKE_UNWIND_WEIGHT> ]
   }

llvm/include/llvm/IR/MDBuilder.h

@@ -59,11 +59,7 @@ class MDBuilder {
   //===------------------------------------------------------------------===//
 
   /// Return metadata containing two branch weights.
-  /// @param TrueWeight the weight of the true branch
-  /// @param FalseWeight the weight of the false branch
-  /// @param Do these weights come from __builtin_expect*
-  MDNode *createBranchWeights(uint32_t TrueWeight, uint32_t FalseWeight,
-                              bool IsExpected = false);
+  MDNode *createBranchWeights(uint32_t TrueWeight, uint32_t FalseWeight);
 
   /// Return metadata containing two branch weights, with significant bias
   /// towards `true` destination.
@@ -74,10 +70,7 @@ class MDBuilder {
   MDNode *createUnlikelyBranchWeights();
 
   /// Return metadata containing a number of branch weights.
-  /// @param Weights the weights of all the branches
-  /// @param Do these weights come from __builtin_expect*
-  MDNode *createBranchWeights(ArrayRef<uint32_t> Weights,
-                              bool IsExpected = false);
+  MDNode *createBranchWeights(ArrayRef<uint32_t> Weights);
 
   /// Return metadata specifying that a branch or switch is unpredictable.
   MDNode *createUnpredictable();

llvm/include/llvm/IR/ProfDataUtils.h

@@ -55,17 +55,6 @@ MDNode *getBranchWeightMDNode(const Instruction &I);
 /// Nullptr otherwise.
 MDNode *getValidBranchWeightMDNode(const Instruction &I);
 
-/// Check if Branch Weight Metadata has an "expected" field from an llvm.expect*
-/// intrinsic
-bool hasBranchWeightOrigin(const Instruction &I);
-
-/// Check if Branch Weight Metadata has an "expected" field from an llvm.expect*
-/// intrinsic
-bool hasBranchWeightOrigin(const MDNode *ProfileData);
-
-/// Return the offset to the first branch weight data
-unsigned getBranchWeightOffset(const MDNode *ProfileData);
-
 /// Extract branch weights from MD_prof metadata
 ///
 /// \param ProfileData A pointer to an MDNode.
@@ -122,11 +111,7 @@ bool extractProfTotalWeight(const Instruction &I, uint64_t &TotalWeights);
 
 /// Create a new `branch_weights` metadata node and add or overwrite
 /// a `prof` metadata reference to instruction `I`.
-/// \param I the Instruction to set branch weights on.
-/// \param Weights an array of weights to set on instruction I.
-/// \param IsExpected were these weights added from an llvm.expect* intrinsic.
-void setBranchWeights(Instruction &I, ArrayRef<uint32_t> Weights,
-                      bool IsExpected);
+void setBranchWeights(Instruction &I, ArrayRef<uint32_t> Weights);
 
 /// Scaling the profile data attached to 'I' using the ratio of S/T.
 void scaleProfData(Instruction &I, uint64_t S, uint64_t T);

llvm/lib/CodeGen/CodeGenPrepare.cpp

@@ -8866,8 +8866,7 @@ bool CodeGenPrepare::splitBranchCondition(Function &F, ModifyDT &ModifiedDT) {
         scaleWeights(NewTrueWeight, NewFalseWeight);
         Br1->setMetadata(LLVMContext::MD_prof,
                          MDBuilder(Br1->getContext())
-                             .createBranchWeights(TrueWeight, FalseWeight,
-                                                  hasBranchWeightOrigin(*Br1)));
+                             .createBranchWeights(TrueWeight, FalseWeight));
 
         NewTrueWeight = TrueWeight;
         NewFalseWeight = 2 * FalseWeight;

llvm/lib/IR/Instruction.cpp

@@ -1268,23 +1268,12 @@ Instruction *Instruction::cloneImpl() const {
 
 void Instruction::swapProfMetadata() {
   MDNode *ProfileData = getBranchWeightMDNode(*this);
-  if (!ProfileData)
-    return;
-  unsigned FirstIdx = getBranchWeightOffset(ProfileData);
-  if (ProfileData->getNumOperands() != 2 + FirstIdx)
+  if (!ProfileData || ProfileData->getNumOperands() != 3)
     return;
 
-  unsigned SecondIdx = FirstIdx + 1;
-  SmallVector<Metadata *, 4> Ops;
-  // If there are more weights past the second, we can't swap them
-  if (ProfileData->getNumOperands() > SecondIdx + 1)
-    return;
-  for (unsigned Idx = 0; Idx < FirstIdx; ++Idx) {
-    Ops.push_back(ProfileData->getOperand(Idx));
-  }
-  // Switch the order of the weights
-  Ops.push_back(ProfileData->getOperand(SecondIdx));
-  Ops.push_back(ProfileData->getOperand(FirstIdx));
+  // The first operand is the name. Fetch them backwards and build a new one.
+  Metadata *Ops[] = {ProfileData->getOperand(0), ProfileData->getOperand(2),
+                     ProfileData->getOperand(1)};
   setMetadata(LLVMContext::MD_prof,
               MDNode::get(ProfileData->getContext(), Ops));
 }

llvm/lib/IR/Instructions.cpp

@@ -5199,11 +5199,7 @@ void SwitchInstProfUpdateWrapper::init() {
   if (!ProfileData)
     return;
 
-  // FIXME: This check belongs in ProfDataUtils. Its almost equivalent to
-  // getValidBranchWeightMDNode(), but the need to use llvm_unreachable
-  // makes them slightly different.
-  if (ProfileData->getNumOperands() !=
-      SI.getNumSuccessors() + getBranchWeightOffset(ProfileData)) {
+  if (ProfileData->getNumOperands() != SI.getNumSuccessors() + 1) {
     llvm_unreachable("number of prof branch_weights metadata operands does "
                      "not correspond to number of succesors");
   }

llvm/lib/IR/MDBuilder.cpp

@@ -35,8 +35,8 @@ MDNode *MDBuilder::createFPMath(float Accuracy) {
 }
 
 MDNode *MDBuilder::createBranchWeights(uint32_t TrueWeight,
-                                       uint32_t FalseWeight, bool IsExpected) {
-  return createBranchWeights({TrueWeight, FalseWeight}, IsExpected);
+                                       uint32_t FalseWeight) {
+  return createBranchWeights({TrueWeight, FalseWeight});
 }
 
 MDNode *MDBuilder::createLikelyBranchWeights() {
@@ -49,19 +49,15 @@ MDNode *MDBuilder::createUnlikelyBranchWeights() {
   return createBranchWeights(1, (1U << 20) - 1);
 }
 
-MDNode *MDBuilder::createBranchWeights(ArrayRef<uint32_t> Weights,
-                                       bool IsExpected) {
+MDNode *MDBuilder::createBranchWeights(ArrayRef<uint32_t> Weights) {
   assert(Weights.size() >= 1 && "Need at least one branch weights!");
 
-  unsigned int Offset = IsExpected ? 2 : 1;
-  SmallVector<Metadata *, 4> Vals(Weights.size() + Offset);
+  SmallVector<Metadata *, 4> Vals(Weights.size() + 1);
   Vals[0] = createString("branch_weights");
-  if (IsExpected)
-    Vals[1] = createString("expected");
 
   Type *Int32Ty = Type::getInt32Ty(Context);
   for (unsigned i = 0, e = Weights.size(); i != e; ++i)
-    Vals[i + Offset] = createConstant(ConstantInt::get(Int32Ty, Weights[i]));
+    Vals[i + 1] = createConstant(ConstantInt::get(Int32Ty, Weights[i]));
 
   return MDNode::get(Context, Vals);
 }

llvm/lib/IR/Metadata.cpp

@@ -1196,10 +1196,10 @@ MDNode *MDNode::mergeDirectCallProfMetadata(MDNode *A, MDNode *B,
   StringRef AProfName = AMDS->getString();
   StringRef BProfName = BMDS->getString();
   if (AProfName == "branch_weights" && BProfName == "branch_weights") {
-    ConstantInt *AInstrWeight = mdconst::dyn_extract<ConstantInt>(
-        A->getOperand(getBranchWeightOffset(A)));
-    ConstantInt *BInstrWeight = mdconst::dyn_extract<ConstantInt>(
-        B->getOperand(getBranchWeightOffset(B)));
+    ConstantInt *AInstrWeight =
+        mdconst::dyn_extract<ConstantInt>(A->getOperand(1));
+    ConstantInt *BInstrWeight =
+        mdconst::dyn_extract<ConstantInt>(B->getOperand(1));
     assert(AInstrWeight && BInstrWeight && "verified by LLVM verifier");
     return MDNode::get(Ctx,
                        {MDHelper.createString("branch_weights"),

llvm/lib/IR/ProfDataUtils.cpp

@@ -40,6 +40,9 @@ namespace {
 // We maintain some constants here to ensure that we access the branch weights
 // correctly, and can change the behavior in the future if the layout changes
 
+// The index at which the weights vector starts
+constexpr unsigned WeightsIdx = 1;
+
 // the minimum number of operands for MD_prof nodes with branch weights
 constexpr unsigned MinBWOps = 3;
 
@@ -72,16 +75,15 @@ static void extractFromBranchWeightMD(const MDNode *ProfileData,
   assert(isBranchWeightMD(ProfileData) && "wrong metadata");
 
   unsigned NOps = ProfileData->getNumOperands();
-  unsigned WeightsIdx = getBranchWeightOffset(ProfileData);
   assert(WeightsIdx < NOps && "Weights Index must be less than NOps.");
   Weights.resize(NOps - WeightsIdx);
 
   for (unsigned Idx = WeightsIdx, E = NOps; Idx != E; ++Idx) {
     ConstantInt *Weight =
         mdconst::dyn_extract<ConstantInt>(ProfileData->getOperand(Idx));
     assert(Weight && "Malformed branch_weight in MD_prof node");
-    assert(Weight->getValue().getActiveBits() <= (sizeof(T) * 8) &&
-           "Too many bits for MD_prof branch_weight");
+    assert(Weight->getValue().getActiveBits() <= 32 &&
+           "Too many bits for uint32_t");
     Weights[Idx - WeightsIdx] = Weight->getZExtValue();
   }
 }
@@ -121,26 +123,6 @@ bool hasValidBranchWeightMD(const Instruction &I) {
   return getValidBranchWeightMDNode(I);
 }
 
-bool hasBranchWeightOrigin(const Instruction &I) {
-  auto *ProfileData = I.getMetadata(LLVMContext::MD_prof);
-  return hasBranchWeightOrigin(ProfileData);
-}
-
-bool hasBranchWeightOrigin(const MDNode *ProfileData) {
-  if (!isBranchWeightMD(ProfileData))
-    return false;
-  auto *ProfDataName = dyn_cast<MDString>(ProfileData->getOperand(1));
-  // NOTE: if we ever have more types of branch weight provenance,
-  // we need to check the string value is "expected". For now, we
-  // supply a more generic API, and avoid the spurious comparisons.
-  assert(ProfDataName == nullptr || ProfDataName->getString() == "expected");
-  return ProfDataName != nullptr;
-}
-
-unsigned getBranchWeightOffset(const MDNode *ProfileData) {
-  return hasBranchWeightOrigin(ProfileData) ? 2 : 1;
-}
-
 MDNode *getBranchWeightMDNode(const Instruction &I) {
   auto *ProfileData = I.getMetadata(LLVMContext::MD_prof);
   if (!isBranchWeightMD(ProfileData))
@@ -150,9 +132,7 @@ MDNode *getBranchWeightMDNode(const Instruction &I) {
 
 MDNode *getValidBranchWeightMDNode(const Instruction &I) {
   auto *ProfileData = getBranchWeightMDNode(I);
-  auto Offset = getBranchWeightOffset(ProfileData);
-  if (ProfileData &&
-      ProfileData->getNumOperands() == Offset + I.getNumSuccessors())
+  if (ProfileData && ProfileData->getNumOperands() == 1 + I.getNumSuccessors())
     return ProfileData;
   return nullptr;
 }
@@ -211,8 +191,7 @@ bool extractProfTotalWeight(const MDNode *ProfileData, uint64_t &TotalVal) {
     return false;
 
   if (ProfDataName->getString() == "branch_weights") {
-    unsigned Offset = getBranchWeightOffset(ProfileData);
-    for (unsigned Idx = Offset; Idx < ProfileData->getNumOperands(); ++Idx) {
+    for (unsigned Idx = 1; Idx < ProfileData->getNumOperands(); Idx++) {
       auto *V = mdconst::dyn_extract<ConstantInt>(ProfileData->getOperand(Idx));
       assert(V && "Malformed branch_weight in MD_prof node");
       TotalVal += V->getValue().getZExtValue();
@@ -233,10 +212,9 @@ bool extractProfTotalWeight(const Instruction &I, uint64_t &TotalVal) {
   return extractProfTotalWeight(I.getMetadata(LLVMContext::MD_prof), TotalVal);
 }
 
-void setBranchWeights(Instruction &I, ArrayRef<uint32_t> Weights,
-                      bool IsExpected) {
+void setBranchWeights(Instruction &I, ArrayRef<uint32_t> Weights) {
   MDBuilder MDB(I.getContext());
-  MDNode *BranchWeights = MDB.createBranchWeights(Weights, IsExpected);
+  MDNode *BranchWeights = MDB.createBranchWeights(Weights);
   I.setMetadata(LLVMContext::MD_prof, BranchWeights);
 }