[InstCombine] Set !prof metadata on Selects identified by add.ll test #158743

alanzhao1 · 2025-09-15T21:53:45Z

These select instructions are created from non-branching instructions, so their branch weights are unknown.

Tracking issue: #147390

These select instructions are created from non-branching instructions, so their branch weights are unknown. Tracking issue: llvm#147390

llvmbot · 2025-09-15T21:54:14Z

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-llvm-transforms

Author: Alan Zhao (alanzhao1)

Changes

These select instructions are created from non-branching instructions, so their branch weights are unknown.

Tracking issue: #147390

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

5 Files Affected:

(modified) llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp (+8-2)
(modified) llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp (+4-1)
(modified) llvm/lib/Transforms/InstCombine/InstructionCombining.cpp (+4-1)
(modified) llvm/test/Transforms/InstCombine/preserve-profile.ll (+39)
(modified) llvm/utils/profcheck-xfail.txt (-2)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index 00951fde0cf8a..a480d96cd4cb9 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -24,6 +24,7 @@
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/IR/PatternMatch.h"
+#include "llvm/IR/ProfDataUtils.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/Value.h"
 #include "llvm/Support/AlignOf.h"
@@ -878,13 +879,18 @@ Instruction *InstCombinerImpl::foldAddWithConstant(BinaryOperator &Add) {
     return BinaryOperator::CreateAdd(Builder.CreateNot(Y), X);
 
   // zext(bool) + C -> bool ? C + 1 : C
+  SelectInst *SI = nullptr;
   if (match(Op0, m_ZExt(m_Value(X))) &&
       X->getType()->getScalarSizeInBits() == 1)
-    return SelectInst::Create(X, InstCombiner::AddOne(Op1C), Op1);
+    SI = SelectInst::Create(X, InstCombiner::AddOne(Op1C), Op1);
   // sext(bool) + C -> bool ? C - 1 : C
   if (match(Op0, m_SExt(m_Value(X))) &&
       X->getType()->getScalarSizeInBits() == 1)
-    return SelectInst::Create(X, InstCombiner::SubOne(Op1C), Op1);
+    SI = SelectInst::Create(X, InstCombiner::SubOne(Op1C), Op1);
+  if (SI) {
+    setExplicitlyUnknownBranchWeights(*SI, DEBUG_TYPE);
+    return SI;
+  }
 
   // ~X + C --> (C-1) - X
   if (match(Op0, m_Not(m_Value(X)))) {
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
index 550f095b26ba4..247f1483e14f5 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
@@ -14,6 +14,7 @@
 #include "llvm/Analysis/InstructionSimplify.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/PatternMatch.h"
+#include "llvm/IR/ProfDataUtils.h"
 #include "llvm/Transforms/InstCombine/InstCombiner.h"
 using namespace llvm;
 using namespace PatternMatch;
@@ -1253,7 +1254,9 @@ Instruction *InstCombinerImpl::visitShl(BinaryOperator &I) {
     // shl (zext i1 X), C1 --> select (X, 1 << C1, 0)
     if (match(Op0, m_ZExt(m_Value(X))) && X->getType()->isIntOrIntVectorTy(1)) {
       auto *NewC = Builder.CreateShl(ConstantInt::get(Ty, 1), C1);
-      return SelectInst::Create(X, NewC, ConstantInt::getNullValue(Ty));
+      auto *SI = SelectInst::Create(X, NewC, ConstantInt::getNullValue(Ty));
+      setExplicitlyUnknownBranchWeights(*SI, DEBUG_TYPE);
+      return SI;
     }
   }
 
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index f0ddd5ca94c5a..957f8bd588857 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -81,6 +81,7 @@
 #include "llvm/IR/Operator.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/PatternMatch.h"
+#include "llvm/IR/ProfDataUtils.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/Use.h"
 #include "llvm/IR/User.h"
@@ -1735,7 +1736,9 @@ Instruction *InstCombinerImpl::foldBinopOfSextBoolToSelect(BinaryOperator &BO) {
   Constant *Zero = ConstantInt::getNullValue(BO.getType());
   Value *TVal = Builder.CreateBinOp(BO.getOpcode(), Ones, C);
   Value *FVal = Builder.CreateBinOp(BO.getOpcode(), Zero, C);
-  return SelectInst::Create(X, TVal, FVal);
+  SelectInst *SI = SelectInst::Create(X, TVal, FVal);
+  setExplicitlyUnknownBranchWeights(*SI, DEBUG_TYPE);
+  return SI;
 }
 
 static Value *simplifyOperationIntoSelectOperand(Instruction &I, SelectInst *SI,
diff --git a/llvm/test/Transforms/InstCombine/preserve-profile.ll b/llvm/test/Transforms/InstCombine/preserve-profile.ll
index dd83805ed3397..0b750fd87d641 100644
--- a/llvm/test/Transforms/InstCombine/preserve-profile.ll
+++ b/llvm/test/Transforms/InstCombine/preserve-profile.ll
@@ -46,9 +46,48 @@ define i32 @NegBin(i1 %C) !prof !0 {
   ret i32 %V
 }
 
+define i32 @select_C_minus_1_or_C_from_bool(i1 %x) {
+; CHECK-LABEL: define i32 @select_C_minus_1_or_C_from_bool(
+; CHECK-SAME: i1 [[X:%.*]]) {
+; CHECK-NEXT:    [[ADD:%.*]] = select i1 [[X]], i32 41, i32 42, !prof [[PROF2:![0-9]+]]
+; CHECK-NEXT:    ret i32 [[ADD]]
+;
+  %ext = sext i1 %x to i32
+  %add = add i32 %ext, 42
+  ret i32 %add
+}
+
+define i5 @and_add(i1 %x, i1 %y) {
+; CHECK-LABEL: define i5 @and_add(
+; CHECK-SAME: i1 [[X:%.*]], i1 [[Y:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i1 [[X]], true
+; CHECK-NEXT:    [[TMP2:%.*]] = and i1 [[Y]], [[TMP1]]
+; CHECK-NEXT:    [[R:%.*]] = select i1 [[TMP2]], i5 -2, i5 0, !prof [[PROF2]]
+; CHECK-NEXT:    ret i5 [[R]]
+;
+  %xz = zext i1 %x to i5
+  %ys = sext i1 %y to i5
+  %sub = add i5 %xz, %ys
+  %r = and i5 %sub, 30
+  ret i5 %r
+}
+
+define i32 @add_zext_zext_i1(i1 %a) {
+; CHECK-LABEL: define i32 @add_zext_zext_i1(
+; CHECK-SAME: i1 [[A:%.*]]) {
+; CHECK-NEXT:    [[ADD:%.*]] = select i1 [[A]], i32 2, i32 0, !prof [[PROF2]]
+; CHECK-NEXT:    ret i32 [[ADD]]
+;
+  %zext = zext i1 %a to i32
+  %add = add i32 %zext, %zext
+  ret i32 %add
+}
+
+
 !0 = !{!"function_entry_count", i64 1000}
 !1 = !{!"branch_weights", i32 2, i32 3}
 ;.
 ; CHECK: [[PROF0]] = !{!"function_entry_count", i64 1000}
 ; CHECK: [[PROF1]] = !{!"branch_weights", i32 2, i32 3}
+; CHECK: [[PROF2]] = !{!"unknown", !"instcombine"}
 ;.
diff --git a/llvm/utils/profcheck-xfail.txt b/llvm/utils/profcheck-xfail.txt
index 482848842aa05..582bb42315ac7 100644
--- a/llvm/utils/profcheck-xfail.txt
+++ b/llvm/utils/profcheck-xfail.txt
@@ -836,8 +836,6 @@ Transforms/InstCombine/2011-02-14-InfLoop.ll
 Transforms/InstCombine/AArch64/sve-intrinsic-sel.ll
 Transforms/InstCombine/AArch64/sve-intrinsic-simplify-binop.ll
 Transforms/InstCombine/AArch64/sve-intrinsic-simplify-shift.ll
-Transforms/InstCombine/add2.ll
-Transforms/InstCombine/add.ll
 Transforms/InstCombine/add-mask.ll
 Transforms/InstCombine/add-shl-mul-umax.ll
 Transforms/InstCombine/add-shl-sdiv-to-srem.ll

mtrofin · 2025-09-15T22:26:37Z

These select instructions are created from non-branching instructions, so their branch weights are unknown.

Tracking issue: #147390

Do we want to just do a "catch'em all" for these cases, like #157599?

alanzhao1 · 2025-09-15T23:01:51Z

These select instructions are created from non-branching instructions, so their branch weights are unknown.
Tracking issue: #147390

Do we want to just do a "catch'em all" for these cases, like #157599?

My original thought while creating this PR is to address these profile unknown bugs one by one to gradually reduce noise (so we can explicitly identify the passes where we cannot preserve profile data), but I'm also OK with a global catch all solution. WDYT?

mtrofin · 2025-09-16T01:20:04Z

These select instructions are created from non-branching instructions, so their branch weights are unknown.
Tracking issue: #147390

Do we want to just do a "catch'em all" for these cases, like #157599?

My original thought while creating this PR is to address these profile unknown bugs one by one to gradually reduce noise (so we can explicitly identify the passes where we cannot preserve profile data), but I'm also OK with a global catch all solution. WDYT?

Ah, I was imagining we do the catch'em all at the end, after the cases where we can reasonably easily compute the profile are handled; if it's easier to make progress this way (i.e. your patch), lgtm!

mtrofin

some nits

llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp

nikic

I'd like this metadata to only be generated if profcheck is enabled. Otherwise it just adds noise to the IR.

mtrofin · 2025-09-16T13:54:15Z

I'd like this metadata to only be generated if profcheck is enabled. Otherwise it just adds noise to the IR.

We could gate inserting unknown on whether the function has an entry count that's also not 0. That way, tests, or code without profile info to begin with, aren't affected; tests under profcheck have it, as intended (because we also set a nonzero function entrycount); and, importantly, production builds (with profile info) have it, too, for non-cold functions, and we can start using ORE for an additional layer of diagnostics.

wdyt?

nikic · 2025-09-16T15:28:23Z

That also sounds reasonable to me. I assume checking for a non-zero function entry count is not particularly expensive?

mtrofin · 2025-09-16T15:39:08Z

That also sounds reasonable to me. I assume checking for a non-zero function entry count is not particularly expensive?

It's fetching a function metadata, and should't be expensive. We can also cache this over the whole pass rather than checking more locally.

nikic · 2025-09-16T19:01:34Z

That also sounds reasonable to me. I assume checking for a non-zero function entry count is not particularly expensive?

It's fetching a function metadata, and should't be expensive. We can also cache this over the whole pass rather than checking more locally.

A cache is probably not needed, so let's start without it.

It would be nice to have a helper for this though, I'd expect this to be a rather common pattern for InstCombine in particular, so if we could just replace SelectInst::Create() with createSelectWithUnknownBranchWeights() that would be nice.

alanzhao1 · 2025-09-17T22:49:54Z

That also sounds reasonable to me. I assume checking for a non-zero function entry count is not particularly expensive?

It's fetching a function metadata, and should't be expensive. We can also cache this over the whole pass rather than checking more locally.

A cache is probably not needed, so let's start without it.

It would be nice to have a helper for this though, I'd expect this to be a rather common pattern for InstCombine in particular, so if we could just replace SelectInst::Create() with createSelectWithUnknownBranchWeights() that would be nice.

Done. I created the function setExplicitlyUnknownBranchWeightsIfProfiled(...) which takes an instruction and adds unknown branch weights if the enclosing function has a nonzero entry count.

mtrofin · 2025-09-24T15:22:21Z

@nikic is this good to go? Thanks!

nikic

I'd still like to see a (InstCombine-specific) helper that allows directly replacing SelectInst::Create with a single call.

llvm/lib/IR/ProfDataUtils.cpp

alanzhao1 · 2025-09-25T20:49:19Z

I'd still like to see a (InstCombine-specific) helper that allows directly replacing SelectInst::Create with a single call.

Done

alanzhao1 · 2025-09-25T20:50:26Z

(sorry for not being super responsive on this PR; I'm currently on a release rotation at my job)

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

…lify createSelectInst(...)

llvm/lib/Transforms/InstCombine/InstCombineInternal.h

llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp

llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

nikic

LGTM, thanks!

llvm/lib/Transforms/InstCombine/InstCombineInternal.h

llvm/include/llvm/IR/ProfDataUtils.h

…llvm#158743) These select instructions are created from non-branching instructions, so their branch weights are unknown. Tracking issue: llvm#147390

[InstCombine] Set !prof metadata on Selects identified by add.ll test

2f5c4f8

These select instructions are created from non-branching instructions, so their branch weights are unknown. Tracking issue: llvm#147390

alanzhao1 requested a review from mtrofin September 15, 2025 21:53

alanzhao1 requested a review from nikic as a code owner September 15, 2025 21:53

llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Sep 15, 2025

mtrofin approved these changes Sep 16, 2025

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp Show resolved Hide resolved

add check for !SI

b42e0c5

nikic requested changes Sep 16, 2025

View reviewed changes

add profile unknown only if the enclosing function has profile counts

cc4e8b4

llvmbot added the llvm:ir label Sep 17, 2025

mtrofin approved these changes Sep 18, 2025

View reviewed changes

mtrofin mentioned this pull request Sep 24, 2025

[profcheck] Add unknown branch weights for inlined strcmp/strncmp #160455

Merged

nikic reviewed Sep 24, 2025

View reviewed changes

llvm/lib/IR/ProfDataUtils.cpp Outdated Show resolved Hide resolved

create InstCombine-specific helper and pass function directly

7c396ad

accidentally negated a conditional, fixed

9764b21

nikic reviewed Sep 26, 2025

View reviewed changes

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp Outdated Show resolved Hide resolved

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp Outdated Show resolved Hide resolved

Add function reference as a member of the InstCombiner class and simp…

2555fce

…lify createSelectInst(...)

nikic reviewed Sep 27, 2025

View reviewed changes

remove unnecessary includes and SelectInst variables

2db6abd

mtrofin mentioned this pull request Sep 29, 2025

[profcheck] Add unknown branch weight for inlined memchr calls. #160964

Merged

nikic approved these changes Sep 29, 2025

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineInternal.h Outdated Show resolved Hide resolved

boomanaiden154 reviewed Sep 29, 2025

View reviewed changes

llvm/include/llvm/IR/ProfDataUtils.h Outdated Show resolved Hide resolved

minor code review comments

9570578

alanzhao1 enabled auto-merge (squash) September 29, 2025 20:03

alanzhao1 merged commit 045e09f into llvm:main Sep 29, 2025
9 checks passed

alanzhao1 deleted the bugfix/add-instcombine-tests branch September 29, 2025 21:27

[InstCombine] Set !prof metadata on Selects identified by add.ll test #158743

[InstCombine] Set !prof metadata on Selects identified by add.ll test #158743

Uh oh!

Conversation

alanzhao1 commented Sep 15, 2025

Uh oh!

llvmbot commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrofin commented Sep 15, 2025

Uh oh!

alanzhao1 commented Sep 15, 2025

Uh oh!

mtrofin commented Sep 16, 2025

Uh oh!

mtrofin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

mtrofin commented Sep 16, 2025

Uh oh!

nikic commented Sep 16, 2025

Uh oh!

mtrofin commented Sep 16, 2025

Uh oh!

nikic commented Sep 16, 2025

Uh oh!

alanzhao1 commented Sep 17, 2025

Uh oh!

mtrofin commented Sep 24, 2025

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alanzhao1 commented Sep 25, 2025

Uh oh!

alanzhao1 commented Sep 25, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

llvmbot commented Sep 15, 2025 •

edited

Loading