Skip to content

Conversation

@boomanaiden154
Copy link
Contributor

#163412 touched this last and directly propagated the profile information. This was not correct for the motivating example:

%a = icmp eq i32 %z, 0
%b = icmp eq i32 %z, 1
%v2 = select i1 %b, i1 true, i1 %pred, !prof !18
%v3 = and i1 %a, %v2

to

%a = icmp eq i32 %z, 0
%v3 = select i1 %a, i1 %pred, i1 false

z == 1 does not imply that z == 0 for i8. In general for the and case, we need a => b, which means that b must be equivalent or more restrictive than a, which means we cannot propagate profile information without additional information on the value distribution of z. For the or case we need !a => b. We again cannot derive profile information for a/!a without additional value distribution information.

\llvm#163412 touched this last and directly propagated the profile
information. This was not correct for the motivating example:

  %a = icmp eq i32 %z, 0
  %b = icmp eq i32 %z, 1
  %v2 = select i1 %b, i1 true, i1 %pred, !prof !18
  %v3 = and i1 %a, %v2

to

  %a = icmp eq i32 %z, 0
  %v3 = select i1 %a, i1 %pred, i1 false

z == 1 does not imply that z == 0 for i8. In general for the and case,
we need a => b, which means that b must be equivalent or more restrictive
than a, which means we cannot propagate profile information without
additional information on the value distribution of z. For the or case
we need !a => b. We again cannot derive profile information for a/!a
without additional value distribution information.
@boomanaiden154 boomanaiden154 requested a review from nikic as a code owner December 4, 2025 21:59
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Dec 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Dec 4, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Aiden Grossman (boomanaiden154)

Changes

#163412 touched this last and directly propagated the profile information. This was not correct for the motivating example:

%a = icmp eq i32 %z, 0
%b = icmp eq i32 %z, 1
%v2 = select i1 %b, i1 true, i1 %pred, !prof !18
%v3 = and i1 %a, %v2

to

%a = icmp eq i32 %z, 0
%v3 = select i1 %a, i1 %pred, i1 false

z == 1 does not imply that z == 0 for i8. In general for the and case, we need a => b, which means that b must be equivalent or more restrictive than a, which means we cannot propagate profile information without additional information on the value distribution of z. For the or case we need !a => b. We again cannot derive profile information for a/!a without additional value distribution information.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp (+3-7)
  • (modified) llvm/test/Transforms/InstCombine/select-safe-impliedcond-transforms.ll (+1-1)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index c9f51e4b294b1..c00551bc1f939 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -3067,14 +3067,10 @@ Instruction *InstCombinerImpl::foldAndOrOfSelectUsingImpliedCond(Value *Op,
          "Op must be either i1 or vector of i1.");
   if (SI.getCondition()->getType() != Op->getType())
     return nullptr;
-  if (Value *V = simplifyNestedSelectsUsingImpliedCond(SI, Op, IsAnd, DL)) {
-    Instruction *MDFrom = nullptr;
-    if (!ProfcheckDisableMetadataFixes)
-      MDFrom = &SI;
-    return SelectInst::Create(
+  if (Value *V = simplifyNestedSelectsUsingImpliedCond(SI, Op, IsAnd, DL))
+    return createSelectInstWithUnknownProfile(
         Op, IsAnd ? V : ConstantInt::getTrue(Op->getType()),
-        IsAnd ? ConstantInt::getFalse(Op->getType()) : V, "", nullptr, MDFrom);
-  }
+        IsAnd ? ConstantInt::getFalse(Op->getType()) : V);
   return nullptr;
 }
 
diff --git a/llvm/test/Transforms/InstCombine/select-safe-impliedcond-transforms.ll b/llvm/test/Transforms/InstCombine/select-safe-impliedcond-transforms.ll
index bc988a9bbab04..d783cbf25de28 100644
--- a/llvm/test/Transforms/InstCombine/select-safe-impliedcond-transforms.ll
+++ b/llvm/test/Transforms/InstCombine/select-safe-impliedcond-transforms.ll
@@ -263,5 +263,5 @@ define i1 @neg_icmp_eq_implies_trunc(i8 %x, i1 %c) {
 !1 = !{!"branch_weights", i32 2, i32 3}
 ;.
 ; CHECK: [[META0:![0-9]+]] = !{!"function_entry_count", i64 1000}
-; CHECK: [[PROF1]] = !{!"branch_weights", i32 2, i32 3}
+; CHECK: [[PROF1]] = !{!"unknown", !"instcombine"}
 ;.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM

@boomanaiden154 boomanaiden154 merged commit a94ee7e into llvm:main Dec 5, 2025
13 checks passed
@boomanaiden154 boomanaiden154 deleted the instcombine-fold-and-or-correct-profile-metadata branch December 5, 2025 17:17
honeygoyal pushed a commit to honeygoyal/llvm-project that referenced this pull request Dec 9, 2025
…lvm#170756)

\llvm#163412 touched this last and directly propagated the profile
information. This was not correct for the motivating example:

  %a = icmp eq i32 %z, 0
  %b = icmp eq i32 %z, 1
  %v2 = select i1 %b, i1 true, i1 %pred, !prof !18
  %v3 = and i1 %a, %v2

to

  %a = icmp eq i32 %z, 0
  %v3 = select i1 %a, i1 %pred, i1 false

z == 1 does not imply that z == 0 for i8. In general for the and case,
we need a => b, which means that b must be equivalent or more
restrictive than a, which means we cannot propagate profile information
without additional information on the value distribution of z. For the
or case we need !a => b. We again cannot derive profile information for
a/!a without additional value distribution information.
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.

3 participants