Skip to content

Conversation

mtrofin
Copy link
Member

@mtrofin mtrofin commented Oct 9, 2025

Follow up from PR #162147. We do not have existing !prof metadata to synthesize one for the new select​ .

Fixes https://lab.llvm.org/staging/#/builders/221/builds/3091

Issue #147390

Copy link
Member Author

mtrofin commented Oct 9, 2025

@mtrofin mtrofin marked this pull request as ready for review October 9, 2025 20:00
@mtrofin mtrofin requested a review from nikic as a code owner October 9, 2025 20:00
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Oct 9, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 9, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Mircea Trofin (mtrofin)

Changes

Follow up from PR #162147.


Full diff: https://github.com/llvm/llvm-project/pull/162726.diff

1 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp (+6)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
index 127a506e440b7..eb03f16e89e03 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
@@ -16,6 +16,7 @@
 #include "llvm/IR/GetElementPtrTypeIterator.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/PatternMatch.h"
+#include "llvm/IR/ProfDataUtils.h"
 #include "llvm/Support/KnownBits.h"
 #include "llvm/Transforms/InstCombine/InstCombiner.h"
 
@@ -108,6 +109,11 @@ static Value *simplifyShiftSelectingPackedElement(Instruction *I,
       IC.Builder.CreateICmpEQ(ShrAmt, Constant::getNullValue(ShrAmt->getType()),
                               ShrAmt->getName() + ".z");
   Value *Select = IC.Builder.CreateSelect(ShrAmtZ, Lower, Upper);
+  // There is no existing !prof metadata we can derive the !prof metadata for
+  // this select.
+  if (auto *SI = dyn_cast<SelectInst>(Select))
+    setExplicitlyUnknownBranchWeightsIfProfiled(
+        *SI, *SI->getParent()->getParent(), DEBUG_TYPE);
   Select->takeName(I);
   return Select;
 }

@mtrofin mtrofin requested a review from dtcxzyw October 9, 2025 20:01
Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No existing data to pull from, and there's no way to have a reasonable guess about which why the select goes without instrumentation.

// this select.
if (auto *SI = dyn_cast<SelectInst>(Select))
setExplicitlyUnknownBranchWeightsIfProfiled(
*SI, *SI->getParent()->getParent(), DEBUG_TYPE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use

SelectInst *createSelectInst(Value *C, Value *S1, Value *S2,
here instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup - just had to make it public (plus the change before in the stack)

@mtrofin mtrofin changed the base branch from main to users/mtrofin/10-09-_nfc_instcombine_make_use_of_unknown_profile_info_clear_in_the_api_name October 10, 2025 03:22
@mtrofin mtrofin force-pushed the users/mtrofin/10-09-_nfc_instcombine_make_use_of_unknown_profile_info_clear_in_the_api_name branch from eda40bc to 8ce46b2 Compare October 10, 2025 03:23
@mtrofin mtrofin force-pushed the users/mtrofin/10-09-_instcombine_mark_as_unknown_the_branch_weights_of_packed_integer_selecting_shifts branch from 4d706b2 to 7be6926 Compare October 10, 2025 03:23
@mtrofin mtrofin force-pushed the users/mtrofin/10-09-_instcombine_mark_as_unknown_the_branch_weights_of_packed_integer_selecting_shifts branch 2 times, most recently from ac32705 to 73e18c0 Compare October 10, 2025 14:59
@mtrofin mtrofin force-pushed the users/mtrofin/10-09-_nfc_instcombine_make_use_of_unknown_profile_info_clear_in_the_api_name branch from 8ce46b2 to f1efbcc Compare October 10, 2025 14:59
Base automatically changed from users/mtrofin/10-09-_nfc_instcombine_make_use_of_unknown_profile_info_clear_in_the_api_name to main October 10, 2025 16:33
@mtrofin mtrofin force-pushed the users/mtrofin/10-09-_instcombine_mark_as_unknown_the_branch_weights_of_packed_integer_selecting_shifts branch from 73e18c0 to 12cae82 Compare October 10, 2025 16:35
Copy link
Member Author

mtrofin commented Oct 10, 2025

Merge activity

  • Oct 10, 5:28 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Oct 10, 5:30 PM UTC: @mtrofin merged this pull request with Graphite.

@mtrofin mtrofin merged commit 01e19e8 into main Oct 10, 2025
10 checks passed
@mtrofin mtrofin deleted the users/mtrofin/10-09-_instcombine_mark_as_unknown_the_branch_weights_of_packed_integer_selecting_shifts branch October 10, 2025 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants