[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 502875.
paulkirth added a comment.

Remove redunant assert


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

Files:
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
  clang/test/CodeGenCXX/attr-likelihood-if-vs-builtin-expect.cpp
  llvm/docs/BranchWeightMetadata.rst
  llvm/include/llvm/IR/MDBuilder.h
  llvm/include/llvm/IR/ProfDataUtils.h
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/IR/Instruction.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/IR/ProfDataUtils.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/lib/Transforms/Utils/SimplifyCFG.cpp
  llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
  llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
  llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll

Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
@@ -235,5 +235,5 @@
   ret void
 }
 
-; CHECK: !0 = !{!"branch_weights", i32 2147483647, i32 1}
-; CHECK: !1 = !{!"branch_weights", i32 1, i32 2147483647}
+; CHECK: !0 = !{!"branch_weights", !"expected", i32 2147483647, i32 1}
+; CHECK: !1 = !{!"branch_weights", !"expected", i32 1, i32 2147483647}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
@@ -53,4 +53,4 @@
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
 
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
@@ -99,5 +99,5 @@
 
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[WEIGHT2]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[WEIGHT2]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
@@ -352,5 +352,5 @@
 !llvm.ident = !{!0}
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[WEIGHT2]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[WEIGHT2]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
@@ -99,6 +99,6 @@
 
 !0 = !{i32 1, !"wchar_size", i32 4}
 !1 = !{!"clang version 5.0.0 (trunk 304373)"}
-; CHECK: [[LIKELY]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[UNLIKELY]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[LIKELY]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[UNLIKELY]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
 
Index: llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
@@ -284,7 +284,7 @@
 
 declare i1 @llvm.expect.with.probability.i1(i1, i1, double) nounwind readnone
 
-; CHECK: !0 = !{!"branch_weights", i32 1717986918, i32 429496731}
-; CHECK: !1 = !{!"branch_weights", i32 429496731, i32 1717986918}
-; CHECK: !2 = !{!"branch_weights", i32 214748366, i32 214748366, i32 1717986918}
-; CHECK: !3 = !{!"branch_weights", i32 1717986918, i32 214748366, i32 214748366}
+; CHECK: !0 = !{!"branch_weights", !"expected", i32 1717986918, i32 429496731}
+; CHECK: !1 = !{!"branch_weights", !"expected", i32 429496731, i32 1717986918}
+; CHECK: !2 = 

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 502873.
paulkirth marked 4 inline comments as done.
paulkirth added a comment.

Rebase and address comments.

- update doc comments and describe `IsExpected` parameter
- add assert when scaling branch weights on calls
- remove redundant check, now done in `getValidBranchWeightMDNode()`
- update assert comment, and describe rationale for `hasExpectedProvenance` in 
`checkBackendInstrumentation()`

TODO:

- investigate making `IsExpected` a non-optional parameter.
- investigate more checks on the instruction type when metadata has an 
`!expected` field


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

Files:
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
  clang/test/CodeGenCXX/attr-likelihood-if-vs-builtin-expect.cpp
  llvm/docs/BranchWeightMetadata.rst
  llvm/include/llvm/IR/MDBuilder.h
  llvm/include/llvm/IR/ProfDataUtils.h
  llvm/lib/Analysis/BranchProbabilityInfo.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/IR/Instruction.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/IR/ProfDataUtils.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/lib/Transforms/Utils/SimplifyCFG.cpp
  llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
  llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
  llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll

Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
@@ -235,5 +235,5 @@
   ret void
 }
 
-; CHECK: !0 = !{!"branch_weights", i32 2147483647, i32 1}
-; CHECK: !1 = !{!"branch_weights", i32 1, i32 2147483647}
+; CHECK: !0 = !{!"branch_weights", !"expected", i32 2147483647, i32 1}
+; CHECK: !1 = !{!"branch_weights", !"expected", i32 1, i32 2147483647}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
@@ -53,4 +53,4 @@
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
 
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
@@ -99,5 +99,5 @@
 
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[WEIGHT2]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[WEIGHT2]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
@@ -352,5 +352,5 @@
 !llvm.ident = !{!0}
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[WEIGHT2]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[WEIGHT2]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
@@ -99,6 +99,6 @@
 
 !0 = !{i32 1, !"wchar_size", i32 4}
 !1 = !{!"clang version 5.0.0 (trunk 304373)"}
-; CHECK: [[LIKELY]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[UNLIKELY]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[LIKELY]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[UNLIKELY]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
 
Index: llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
@@ -284,7 +284,7 @@
 
 declare i1 

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added inline comments.



Comment at: llvm/include/llvm/IR/MDBuilder.h:61
 
   /// Return metadata containing two branch weights.
+  MDNode *createBranchWeights(uint32_t TrueWeight, uint32_t FalseWeight,

tejohnson wrote:
> Update comment to mention new parameter in this and the below methods.
> 
> Also, I wonder if it would be better to make the IsExpected non-optional, to 
> force any new callers to think about whether they need that and help avoid 
> issues where it gets dropped incorrectly?
> Also, I wonder if it would be better to make the IsExpected non-optional, to 
> force any new callers to think about whether they need that and help avoid 
> issues where it gets dropped incorrectly?

I think that's a good idea. I can see how much that affects this patch after 
addressing the current set of comments.




Comment at: llvm/lib/Analysis/BranchProbabilityInfo.cpp:394
+  // Ensure there are weights for all of the successors. Note that the first
+  // operand to the metadata node is a name, not a weight.
+  if (WeightsNode->getNumOperands() !=

tejohnson wrote:
> Isn't it now the first 1 or 2 are a name?
Actually, this code block doesn' need to be here at all anymore. That check was 
moved up into `getValidBranchWeightMDNode`. I must  have missed that when I 
rebased.



Comment at: llvm/lib/Analysis/BranchProbabilityInfo.cpp:397
+  TI->getNumSuccessors() + getBranchWeightOffset(WeightsNode))
+return false;
+

tejohnson wrote:
> Should this be an assert?
Same as above. This wasn't an assert originally, so I didn't change its 
behavior.



Comment at: llvm/lib/IR/Verifier.cpp:4553
   StringRef ProfName = MDS->getString();
+  unsigned Offset = getBranchWeightOffset(I);
 

tejohnson wrote:
> Related to an earlier comment regarding calls - maybe the verifier should 
> ensure that "expected" only shows up on certain instruction types?
I think the issue is that we've overload "branch_weights" for lots of things 
that aren't branch weights, but are similar.  IMO it should be something like: 
`!{!"call_count", i32 2000}`, but that likely would break a number of things in 
the codebase, and require lots of special handling for the various different 
uses. Right now the IR change I've made doesn't really disambiguate the various 
cases and just treats them all uniformly.

I'll take a look at updating the verifier rules after addressing the other 
comments.



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:156
+  // weights are being added multiple times, as is the case for SampleProfiling
+  // under ThinLTO. We gate all known entry paths to verifyMisExpect() by first
+  // checking for the presence of the "expected" tag in the metadata, which is

tejohnson wrote:
> Why not check it within verifyMisExpect, seems less error prone than 
> requiring callers to check?
Centralizing the checking would be ideal, but because Frontend and Backend 
checking are not completely symmetric, we cannot rely on the instruction having 
the `Expected` flag set in the metadata when `verifyMisExpect` is called for 
Frontend based instrumentation. 

Maybe I should update the comment to reflect that the "provenance" check 
ensures we won't run backend checks multiple times, and put a more explicit 
comment about it in `checkBackendInstrumentation`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

Sorry for the slow response. Looks pretty good, just a few minor suggestions 
and questions.




Comment at: llvm/include/llvm/IR/MDBuilder.h:61
 
   /// Return metadata containing two branch weights.
+  MDNode *createBranchWeights(uint32_t TrueWeight, uint32_t FalseWeight,

Update comment to mention new parameter in this and the below methods.

Also, I wonder if it would be better to make the IsExpected non-optional, to 
force any new callers to think about whether they need that and help avoid 
issues where it gets dropped incorrectly?



Comment at: llvm/include/llvm/IR/ProfDataUtils.h:58
 
+/// Check if Branch Weight Metadata has an "expected" field
+bool hasExpectedProvenance(const Instruction );

Maybe note the meaning of the expected field (i.e. had an llvm.expect 
intrinsic).



Comment at: llvm/lib/Analysis/BranchProbabilityInfo.cpp:394
+  // Ensure there are weights for all of the successors. Note that the first
+  // operand to the metadata node is a name, not a weight.
+  if (WeightsNode->getNumOperands() !=

Isn't it now the first 1 or 2 are a name?



Comment at: llvm/lib/Analysis/BranchProbabilityInfo.cpp:397
+  TI->getNumSuccessors() + getBranchWeightOffset(WeightsNode))
+return false;
+

Should this be an assert?



Comment at: llvm/lib/IR/Instructions.cpp:732
   ProfileData->getNumOperands() > 0) {
+auto Offset = getBranchWeightOffset(ProfileData);
 // Using APInt::div may be expensive, but most cases should fit 64 bits.

I guess we don't have to worry about pushing the expect provenance because this 
is a call and it shouldn't have one. Maybe add an assert that the offset is 1?



Comment at: llvm/lib/IR/Verifier.cpp:4553
   StringRef ProfName = MDS->getString();
+  unsigned Offset = getBranchWeightOffset(I);
 

Related to an earlier comment regarding calls - maybe the verifier should 
ensure that "expected" only shows up on certain instruction types?



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:156
+  // weights are being added multiple times, as is the case for SampleProfiling
+  // under ThinLTO. We gate all known entry paths to verifyMisExpect() by first
+  // checking for the presence of the "expected" tag in the metadata, which is

Why not check it within verifyMisExpect, seems less error prone than requiring 
callers to check?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 492493.
paulkirth edited the summary of this revision.
paulkirth added a comment.

Update summary


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

Files:
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
  clang/test/CodeGenCXX/attr-likelihood-if-vs-builtin-expect.cpp
  llvm/docs/BranchWeightMetadata.rst
  llvm/include/llvm/IR/MDBuilder.h
  llvm/include/llvm/IR/ProfDataUtils.h
  llvm/lib/Analysis/BranchProbabilityInfo.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/IR/Instruction.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/IR/ProfDataUtils.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/lib/Transforms/Utils/SimplifyCFG.cpp
  llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
  llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
  llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll

Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
@@ -235,5 +235,5 @@
   ret void
 }
 
-; CHECK: !0 = !{!"branch_weights", i32 2147483647, i32 1}
-; CHECK: !1 = !{!"branch_weights", i32 1, i32 2147483647}
+; CHECK: !0 = !{!"branch_weights", !"expected", i32 2147483647, i32 1}
+; CHECK: !1 = !{!"branch_weights", !"expected", i32 1, i32 2147483647}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
@@ -53,4 +53,4 @@
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
 
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
@@ -99,5 +99,5 @@
 
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[WEIGHT2]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[WEIGHT2]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
@@ -352,5 +352,5 @@
 !llvm.ident = !{!0}
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[WEIGHT2]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[WEIGHT2]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
@@ -99,6 +99,6 @@
 
 !0 = !{i32 1, !"wchar_size", i32 4}
 !1 = !{!"clang version 5.0.0 (trunk 304373)"}
-; CHECK: [[LIKELY]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[UNLIKELY]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[LIKELY]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[UNLIKELY]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
 
Index: llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
@@ -284,7 +284,7 @@
 
 declare i1 @llvm.expect.with.probability.i1(i1, i1, double) nounwind readnone
 
-; CHECK: !0 = !{!"branch_weights", i32 1717986918, i32 429496731}
-; CHECK: !1 = !{!"branch_weights", i32 429496731, i32 1717986918}
-; CHECK: !2 = !{!"branch_weights", i32 214748366, i32 214748366, i32 1717986918}
-; CHECK: !3 = !{!"branch_weights", i32 1717986918, i32 214748366, i32 214748366}
+; CHECK: !0 = !{!"branch_weights", !"expected", i32 1717986918, i32 429496731}
+; CHECK: !1 = 

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 492492.
paulkirth added a comment.

Rebase and update lang ref


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

Files:
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
  clang/test/CodeGenCXX/attr-likelihood-if-vs-builtin-expect.cpp
  llvm/docs/BranchWeightMetadata.rst
  llvm/include/llvm/IR/MDBuilder.h
  llvm/include/llvm/IR/ProfDataUtils.h
  llvm/lib/Analysis/BranchProbabilityInfo.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/IR/Instruction.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/IR/ProfDataUtils.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/lib/Transforms/Utils/SimplifyCFG.cpp
  llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
  llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
  llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll

Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
@@ -235,5 +235,5 @@
   ret void
 }
 
-; CHECK: !0 = !{!"branch_weights", i32 2147483647, i32 1}
-; CHECK: !1 = !{!"branch_weights", i32 1, i32 2147483647}
+; CHECK: !0 = !{!"branch_weights", !"expected", i32 2147483647, i32 1}
+; CHECK: !1 = !{!"branch_weights", !"expected", i32 1, i32 2147483647}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
@@ -53,4 +53,4 @@
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
 
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
@@ -99,5 +99,5 @@
 
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[WEIGHT2]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[WEIGHT2]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
@@ -352,5 +352,5 @@
 !llvm.ident = !{!0}
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[WEIGHT2]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[WEIGHT2]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
@@ -99,6 +99,6 @@
 
 !0 = !{i32 1, !"wchar_size", i32 4}
 !1 = !{!"clang version 5.0.0 (trunk 304373)"}
-; CHECK: [[LIKELY]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[UNLIKELY]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[LIKELY]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[UNLIKELY]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
 
Index: llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
@@ -284,7 +284,7 @@
 
 declare i1 @llvm.expect.with.probability.i1(i1, i1, double) nounwind readnone
 
-; CHECK: !0 = !{!"branch_weights", i32 1717986918, i32 429496731}
-; CHECK: !1 = !{!"branch_weights", i32 429496731, i32 1717986918}
-; CHECK: !2 = !{!"branch_weights", i32 214748366, i32 214748366, i32 1717986918}
-; CHECK: !3 = !{!"branch_weights", i32 1717986918, i32 214748366, i32 214748366}
+; CHECK: !0 = !{!"branch_weights", !"expected", i32 1717986918, i32 429496731}
+; CHECK: !1 = !{!"branch_weights", !"expected", i32 429496731, 

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

In D131306#4081864 , @jdoerfert wrote:

> Drive by: There should be a lang ref component to this.

Thanks for pointing out that we need to update the langref.  Offhand, do you 
recall if there are places other than `BranchWeightMetadata.rst` that would 
need to be updated?

> Also, changing branch weight metadata might impact downstream users, an RFC 
> seems in order (assuming I didn't miss one).

You didn't miss one. I was under the impression that the layout of `MD_prof` 
was an internal detail that wouldn't require one. I think that is doubly true 
since the new field is optional, and existing bitcode/IR can still be consumed.

But you raise a fair point that it would impact anyone trying to process new 
IR, so maybe one is in order to be a good citizen.  I'll try to set aside some 
time to draft a short RFC for discourse.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Drive by: There should be a lang ref component to this. Also, changing branch 
weight metadata might impact downstream users, an RFC seems in order (assuming 
I didn't miss one).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 492246.
paulkirth added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

Files:
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
  clang/test/CodeGenCXX/attr-likelihood-if-vs-builtin-expect.cpp
  llvm/include/llvm/IR/MDBuilder.h
  llvm/include/llvm/IR/ProfDataUtils.h
  llvm/lib/Analysis/BranchProbabilityInfo.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/IR/Instruction.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/IR/ProfDataUtils.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/lib/Transforms/Utils/SimplifyCFG.cpp
  llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
  llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
  llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll

Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
@@ -235,5 +235,5 @@
   ret void
 }
 
-; CHECK: !0 = !{!"branch_weights", i32 2147483647, i32 1}
-; CHECK: !1 = !{!"branch_weights", i32 1, i32 2147483647}
+; CHECK: !0 = !{!"branch_weights", !"expected", i32 2147483647, i32 1}
+; CHECK: !1 = !{!"branch_weights", !"expected", i32 1, i32 2147483647}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
@@ -53,4 +53,4 @@
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
 
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
@@ -99,5 +99,5 @@
 
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[WEIGHT2]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[WEIGHT2]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
@@ -352,5 +352,5 @@
 !llvm.ident = !{!0}
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[WEIGHT2]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[WEIGHT2]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
@@ -99,6 +99,6 @@
 
 !0 = !{i32 1, !"wchar_size", i32 4}
 !1 = !{!"clang version 5.0.0 (trunk 304373)"}
-; CHECK: [[LIKELY]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[UNLIKELY]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[LIKELY]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[UNLIKELY]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
 
Index: llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
@@ -284,7 +284,7 @@
 
 declare i1 @llvm.expect.with.probability.i1(i1, i1, double) nounwind readnone
 
-; CHECK: !0 = !{!"branch_weights", i32 1717986918, i32 429496731}
-; CHECK: !1 = !{!"branch_weights", i32 429496731, i32 1717986918}
-; CHECK: !2 = !{!"branch_weights", i32 214748366, i32 214748366, i32 1717986918}
-; CHECK: !3 = !{!"branch_weights", i32 1717986918, i32 214748366, i32 214748366}
+; CHECK: !0 = !{!"branch_weights", !"expected", i32 1717986918, i32 429496731}
+; CHECK: !1 = !{!"branch_weights", !"expected", i32 429496731, i32 1717986918}
+; CHECK: !2 = !{!"branch_weights", 

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 489176.
paulkirth added a comment.

Rebase.

- fix test cases
- improve code swaping branch weights in instruction.cpp
- clean up some dead metadata types left over from an old version of this patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

Files:
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
  clang/test/CodeGenCXX/attr-likelihood-if-vs-builtin-expect.cpp
  llvm/include/llvm/IR/MDBuilder.h
  llvm/include/llvm/IR/ProfDataUtils.h
  llvm/lib/Analysis/BranchProbabilityInfo.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/IR/Instruction.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/IR/ProfDataUtils.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/Scalar/LoopPredication.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/lib/Transforms/Utils/SimplifyCFG.cpp
  llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
  llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
  llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll

Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
@@ -235,5 +235,5 @@
   ret void
 }
 
-; CHECK: !0 = !{!"branch_weights", i32 2147483647, i32 1}
-; CHECK: !1 = !{!"branch_weights", i32 1, i32 2147483647}
+; CHECK: !0 = !{!"branch_weights", !"expected", i32 2147483647, i32 1}
+; CHECK: !1 = !{!"branch_weights", !"expected", i32 1, i32 2147483647}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
@@ -53,4 +53,4 @@
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
 
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
@@ -99,5 +99,5 @@
 
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[WEIGHT2]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[WEIGHT2]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
@@ -352,5 +352,5 @@
 !llvm.ident = !{!0}
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[WEIGHT2]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[WEIGHT2]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
@@ -99,6 +99,6 @@
 
 !0 = !{i32 1, !"wchar_size", i32 4}
 !1 = !{!"clang version 5.0.0 (trunk 304373)"}
-; CHECK: [[LIKELY]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[UNLIKELY]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[LIKELY]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[UNLIKELY]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
 
Index: llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
@@ -284,7 +284,7 @@
 
 declare i1 @llvm.expect.with.probability.i1(i1, i1, double) nounwind readnone
 
-; CHECK: !0 = !{!"branch_weights", i32 1717986918, i32 429496731}
-; CHECK: !1 = !{!"branch_weights", i32 429496731, i32 1717986918}
-; CHECK: !2 = !{!"branch_weights", i32 214748366, i32 214748366, i32 1717986918}
-; CHECK: !3 = !{!"branch_weights", i32 1717986918, i32 214748366, i32 

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D131306#4052878 , @paulkirth wrote:

> In D131306#4052782 , @tejohnson 
> wrote:
>
>> In D131306#4037037 , @paulkirth 
>> wrote:
>>
>>> @tejohnson @xur I kind of dropped the ball on these patches, but what are 
>>> your thoughts on this approach over the old(more invasive) change to the 
>>> profdata format I had prototyped before? the patch will obviously need to 
>>> be rebased, but other than that, do we see a downside to handling 
>>> provenance tracking for branch weights this way?
>>
>> Sorry, it looks like you were waiting on a review of the latest changes from 
>> me but I didn't get to it. I don't recall the other changes you prototyped 
>> off the top of my head - can you point me to that?
>
> It was just the last revision of this patch 
> https://reviews.llvm.org/D131306?vs=on=450448#toc. The way I handled it 
> before was to leave the MD_prof layout alone and use a new MD type to track 
> the provenance. It had the benefit of leaving the layout alone, and the 
> downside that //every// place that did something w/ MD_prof needed to copy 
> that as well.

Ok, yeah, I definitely prefer it to not be a separate metadata that we need to 
keep in sync. I prefer this approach.

>> But I don't have an issue with this approach as I recall it seemed cleanest 
>> at the time.
>
> Sounds good. I'll start work updating this then. Thanks!




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

In D131306#4052782 , @tejohnson wrote:

> In D131306#4037037 , @paulkirth 
> wrote:
>
>> @tejohnson @xur I kind of dropped the ball on these patches, but what are 
>> your thoughts on this approach over the old(more invasive) change to the 
>> profdata format I had prototyped before? the patch will obviously need to be 
>> rebased, but other than that, do we see a downside to handling provenance 
>> tracking for branch weights this way?
>
> Sorry, it looks like you were waiting on a review of the latest changes from 
> me but I didn't get to it. I don't recall the other changes you prototyped 
> off the top of my head - can you point me to that?

It was just the last revision of this patch 
https://reviews.llvm.org/D131306?vs=on=450448#toc. The way I handled it 
before was to leave the MD_prof layout alone and use a new MD type to track the 
provenance. It had the benefit of leaving the layout alone, and the downside 
that //every// place that did something w/ MD_prof needed to copy that as well.

> But I don't have an issue with this approach as I recall it seemed cleanest 
> at the time.

Sounds good. I'll start work updating this then. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D131306#4037037 , @paulkirth wrote:

> @tejohnson @xur I kind of dropped the ball on these patches, but what are 
> your thoughts on this approach over the old(more invasive) change to the 
> profdata format I had prototyped before? the patch will obviously need to be 
> rebased, but other than that, do we see a downside to handling provenance 
> tracking for branch weights this way?

Sorry, it looks like you were waiting on a review of the latest changes from me 
but I didn't get to it. I don't recall the other changes you prototyped off the 
top of my head - can you point me to that? But I don't have an issue with this 
approach as I recall it seemed cleanest at the time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

@tejohnson @xur I kind of dropped the ball on these patches, but what are your 
thoughts on this approach over the old(more invasive) change to the profdata 
format I had prototyped before? the patch will obviously need to be rebased, 
but other than that, do we see a downside to handling provenance tracking for 
branch weights this way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 464332.
paulkirth added a comment.

rebase and change implementation to include provenance information directly in 
the MD_prof metadata


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

Files:
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
  clang/test/CodeGenCXX/attr-likelihood-if-vs-builtin-expect.cpp
  llvm/include/llvm/IR/FixedMetadataKinds.def
  llvm/include/llvm/IR/MDBuilder.h
  llvm/include/llvm/IR/ProfDataUtils.h
  llvm/lib/Analysis/BranchProbabilityInfo.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/IR/Instruction.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/IR/ProfDataUtils.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  llvm/lib/Transforms/Scalar/LoopPredication.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/lib/Transforms/Utils/SimplifyCFG.cpp
  llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
  llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
  llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
  llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll

Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_tern.ll
@@ -53,4 +53,4 @@
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
 
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_or.ll
@@ -99,5 +99,5 @@
 
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[WEIGHT2]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[WEIGHT2]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_merge.ll
@@ -352,5 +352,5 @@
 !llvm.ident = !{!0}
 
 !0 = !{!"clang version 5.0.0 (trunk 302965)"}
-; CHECK: [[WEIGHT]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[WEIGHT2]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[WEIGHT]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[WEIGHT2]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
Index: llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
@@ -99,6 +99,6 @@
 
 !0 = !{i32 1, !"wchar_size", i32 4}
 !1 = !{!"clang version 5.0.0 (trunk 304373)"}
-; CHECK: [[LIKELY]] = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: [[UNLIKELY]] = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: [[LIKELY]] = !{!"branch_weights", !"expected", i32 2000, i32 1}
+; CHECK: [[UNLIKELY]] = !{!"branch_weights", !"expected", i32 1, i32 2000}
 
Index: llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
@@ -285,7 +285,7 @@
 
 declare i1 @llvm.expect.with.probability.i1(i1, i1, double) nounwind readnone
 
-; CHECK: !0 = !{!"branch_weights", i32 1717986918, i32 429496731}
-; CHECK: !1 = !{!"branch_weights", i32 429496731, i32 1717986918}
-; CHECK: !2 = !{!"branch_weights", i32 214748366, i32 214748366, i32 1717986918}
-; CHECK: !3 = !{!"branch_weights", i32 1717986918, i32 214748366, i32 214748366}
+; CHECK: !0 = !{!"branch_weights", !"expected", i32 1717986918, i32 429496731}
+; CHECK: !1 = !{!"branch_weights", !"expected", i32 429496731, i32 1717986918}
+; CHECK: !2 = !{!"branch_weights", !"expected", i32 214748366, i32 214748366, i32 1717986918}
+; CHECK: !3 = !{!"branch_weights", !"expected", i32 1717986918, i32 214748366, i32 214748366}
Index: llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
+++ 

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Rong Xu via Phabricator via cfe-commits]

xur added a comment.

I agreed with Teresa: adding an optional string is better than using a 
separated metadata here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

I think that may be a better approach. Let me investigate a bit and see what 
the exact tradeoffs are.

If anyone else has a suggestion or thought on another way to handle this, I'm 
all ears.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D131306#3756153 , @paulkirth wrote:

> In D131306#3756087 , @tejohnson 
> wrote:
>
>> Well I was thinking the extra field would be optional as well and could be 
>> removed. But understood that this requires more changes (although maybe not 
>> if it is optional, and after your recent changes to centralize some of the 
>> prof metadata handling in the compiler).
>
> Hmm, I don't think I considered that a field in the metadata could be 
> optional. Do you mean something like this?
>
>   !{!"branch_weights", !10, i32 1717986918, i32 429496731}
>
> where `!10` is just some optional metadata, and we'd just ensure things that 
> parse the MD_prof data skip it correctly? Given that we've mostly 
> consolidated how branch weights are extracted and manipulated that might only 
> require a limited number of updates to the code and tests.

Yep! Or a string. I was thinking it might be easier at the end of the list of 
fields, but either way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

In D131306#3756087 , @tejohnson wrote:

> Well I was thinking the extra field would be optional as well and could be 
> removed. But understood that this requires more changes (although maybe not 
> if it is optional, and after your recent changes to centralize some of the 
> prof metadata handling in the compiler).

Hmm, I don't think I considered that a field in the metadata could be optional. 
Do you mean something like this?

  !{!"branch_weights", !10, i32 1717986918, i32 429496731}

where `!10` is just some optional metadata, and we'd just ensure things that 
parse the MD_prof data skip it correctly? Given that we've mostly consolidated 
how branch weights are extracted and manipulated that might only require a 
limited number of updates to the code and tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D131306#3756074 , @paulkirth wrote:

> In D131306#3756009 , @tejohnson 
> wrote:
>
>> @davidxl @xur for review and thoughts.
>>
>> I'm a little wary of requiring that both pieces of metadata be carried 
>> together, as that seems very brittle to maintain in the compiler. What would 
>> happen if the MD_expected didn't get propagated by some pass along with the 
>> MD_prof? I think you would get a false negative, which I suppose is better 
>> than a false positive. An alternative, that I guess would require more 
>> extensive changes, is to add an additional item to the "branch_weights" list 
>> (would need to be obviously distinguishable by type from the list of weights 
>> since that can be variable though).
>
> Agreed. This isn't my preferred solution, but it seemed far less invasive 
> than changing the format of profiling metadata. Originally, I had looked into 
> adding a provenance field to the metadata. It required changes to every test 
> that has branch weights, and I balked at submitting that. I'm also a little 
> wary of making a heavily used metadata type larger. I guess there isn't a lot 
> of difference, but the external metadata is optional, and we could remove it 
> after checking is complete.

Well I was thinking the extra field would be optional as well and could be 
removed. But understood that this requires more changes (although maybe not if 
it is optional, and after your recent changes to centralize some of the prof 
metadata handling in the compiler).

> Unfortunately, I just don't think there is a clean solution here. Either we 
> make an invasive change to the metadata format, or we deal w/ updating 2 
> pieces of metadata everywhere.  I'm just very unsure about which is the right 
> tradeoff.
>
>> Patch needs tests showing uses of the new metadata, and some documentation 
>> in LangRef (i.e. near https://llvm.org/docs/LangRef.html#prof-metadata).
>
> Yes, testing and documentation is something I plan to improve, but I wanted 
> to get some feedback on this approach before investing too heavily.

Ok understood. Hopefully others will chime in with feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.



In D131306#3756009 , @tejohnson wrote:

> @davidxl @xur for review and thoughts.
>
> I'm a little wary of requiring that both pieces of metadata be carried 
> together, as that seems very brittle to maintain in the compiler. What would 
> happen if the MD_expected didn't get propagated by some pass along with the 
> MD_prof? I think you would get a false negative, which I suppose is better 
> than a false positive. An alternative, that I guess would require more 
> extensive changes, is to add an additional item to the "branch_weights" list 
> (would need to be obviously distinguishable by type from the list of weights 
> since that can be variable though).

Agreed. This isn't my preferred solution, but it seemed far less invasive than 
changing the format of profiling metadata. Originally, I had looked into adding 
a provenance field to the metadata. It required changes to every test that has 
branch weights, and I balked at submitting that. I'm also a little wary of 
making a heavily used metadata type larger. I guess there isn't a lot of 
difference, but the external metadata is optional, and we could remove it after 
checking is complete.

Unfortunately, I just don't think there is a clean solution here. Either we 
make an invasive change to the metadata format, or we deal w/ updating 2 pieces 
of metadata everywhere.  I'm just very unsure about which is the right tradeoff.

> Patch needs tests showing uses of the new metadata, and some documentation in 
> LangRef (i.e. near https://llvm.org/docs/LangRef.html#prof-metadata).

Yes, testing and documentation is something I plan to improve, but I wanted to 
get some feedback on this approach before investing too heavily.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added subscribers: xur, davidxl.
tejohnson added a comment.

@davidxl @xur for review and thoughts.

I'm a little wary of requiring that both pieces of metadata be carried 
together, as that seems very brittle to maintain in the compiler. What would 
happen if the MD_expected didn't get propagated by some pass along with the 
MD_prof? I think you would get a false negative, which I suppose is better than 
a false positive. An alternative, that I guess would require more extensive 
changes, is to add an additional item to the "branch_weights" list (would need 
to be obviously distinguishable by type from the list of weights since that can 
be variable though).

Patch needs tests showing uses of the new metadata, and some documentation in 
LangRef (i.e. near https://llvm.org/docs/LangRef.html#prof-metadata).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131306

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

[Paul Kirth via Phabricator via cfe-commits]

paulkirth created this revision.
paulkirth added a reviewer: tejohnson.
Herald added subscribers: ormris, okura, kuter, hiraditya.
Herald added a project: All.
paulkirth requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: sstefan1.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

MisExpect needs to know if a branch weight intrinsic originates from an
llvm.expect intrinsic.

This patch allows us to track that provenance by adding a new
metadata type that can be moved in concert with the existing branch
weights. The new metadata is copied whenever branch weights are copied,
and is removed when new branch weights are added.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131306

Files:
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
  clang/test/CodeGenCXX/attr-likelihood-if-vs-builtin-expect.cpp
  llvm/include/llvm/IR/FixedMetadataKinds.def
  llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
  llvm/lib/Transforms/IPO/LowerTypeTests.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
  llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll

Index: llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll
@@ -138,7 +138,7 @@
   %conv1 = sext i32 %conv to i64
   %expval = call i64 @llvm.expect.with.probability.i64(i64 %conv1, i64 0, double 8.00e-01)
   %tobool = icmp ne i64 %expval, 0
-; CHECK: !prof !1
+; CHECK: !prof !2
 ; CHECK-NOT: @llvm.expect.with.probability
   br i1 %tobool, label %if.then, label %if.end
 
@@ -165,7 +165,7 @@
   %tmp = load i32, i32* %x.addr, align 4
   %conv = sext i32 %tmp to i64
   %expval = call i64 @llvm.expect.with.probability.i64(i64 %conv, i64 2, double 8.00e-01)
-; CHECK: !prof !2
+; CHECK: !prof !3
 ; CHECK-NOT: @llvm.expect.with.probability
   switch i64 %expval, label %sw.epilog [
 i64 1, label %sw.bb
@@ -194,7 +194,7 @@
   %tmp = load i32, i32* %x.addr, align 4
   %conv = sext i32 %tmp to i64
   %expval = call i64 @llvm.expect.with.probability.i64(i64 %conv, i64 1, double 8.00e-01)
-; CHECK: !prof !3
+; CHECK: !prof !4
 ; CHECK-NOT: @llvm.expect.with.probability
   switch i64 %expval, label %sw.epilog [
 i64 2, label %sw.bb
@@ -278,7 +278,7 @@
   %t7 = call i64 @llvm.expect.with.probability.i64(i64 %t6, i64 0, double 8.00e-01)
   %t8 = icmp ne i64 %t7, 0
   %t9 = select i1 %t8, i32 1, i32 2
-; CHECK: select{{.*}}, !prof !1
+; CHECK: select{{.*}}, !prof !2
   ret i32 %t9
 }
 
@@ -286,6 +286,6 @@
 declare i1 @llvm.expect.with.probability.i1(i1, i1, double) nounwind readnone
 
 ; CHECK: !0 = !{!"branch_weights", i32 1717986918, i32 429496731}
-; CHECK: !1 = !{!"branch_weights", i32 429496731, i32 1717986918}
-; CHECK: !2 = !{!"branch_weights", i32 214748366, i32 214748366, i32 1717986918}
-; CHECK: !3 = !{!"branch_weights", i32 1717986918, i32 214748366, i32 214748366}
+; CHECK: !2 = !{!"branch_weights", i32 429496731, i32 1717986918}
+; CHECK: !3 = !{!"branch_weights", i32 214748366, i32 214748366, i32 1717986918}
+; CHECK: !4 = !{!"branch_weights", i32 1717986918, i32 214748366, i32 214748366}
Index: llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
@@ -138,7 +138,7 @@
   %conv1 = sext i32 %conv to i64
   %expval = call i64 @llvm.expect.i64(i64 %conv1, i64 0)
   %tobool = icmp ne i64 %expval, 0
-; CHECK: !prof !1
+; CHECK: !prof !2
 ; CHECK-NOT: @llvm.expect
   br i1 %tobool, label %if.then, label %if.end
 
@@ -165,7 +165,7 @@
   %tmp = load i32, i32* %x.addr, align 4
   %conv = sext i32 %tmp to i64
   %expval = call i64 @llvm.expect.i64(i64 %conv, i64 2)
-; CHECK: !prof !2
+; CHECK: !prof !3
 ; CHECK-NOT: @llvm.expect
   switch i64 %expval, label %sw.epilog [
 i64 1, label %sw.bb
@@ -194,7 +194,7 @@
   %tmp = load i32, i32* %x.addr, align 4
   %conv = sext i32 %tmp to i64
   %expval = call i64 @llvm.expect.i64(i64 %conv, i64 1)
-; CHECK: !prof !3
+; CHECK: !prof !4
 ; CHECK-NOT: @llvm.expect
   switch i64 %expval, label %sw.epilog [
 i64 2, label %sw.bb
@@ -278,7 +278,7 @@
   %t7 = call i64 @llvm.expect.i64(i64 %t6, i64 0)
   %t8 = icmp ne i64 %t7, 0
   %t9 = select i1 %t8, i32 1, i32 2
-; CHECK: select{{.*}}, !prof !1
+; CHECK: select{{.*}}, !prof !2
   ret i32 %t9
 }