[InstCombine] Add one-use check when folding fabs over selects #122270
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Jan 9, 2025
llvmbot
added
llvm:instcombine
|
@llvm/pr-subscribers-llvm-transforms Author: Yingwei Zheng (dtcxzyw) ChangesFixes multi-use issue introduced by #86390. Full diff: https://github.com/llvm/llvm-project/pull/122270.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index c55c40c88bc845..a19eded6565997 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -2709,7 +2709,8 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
if (match(Arg, m_Select(m_Value(Cond), m_Value(TVal), m_Value(FVal)))) {
// fabs (select Cond, TrueC, FalseC) --> select Cond, AbsT, AbsF
- if (isa<Constant>(TVal) || isa<Constant>(FVal)) {
+ if (Arg->hasOneUse() ? (isa<Constant>(TVal) || isa<Constant>(FVal))
+ : (isa<Constant>(TVal) && isa<Constant>(FVal))) {
CallInst *AbsT = Builder.CreateCall(II->getCalledFunction(), {TVal});
CallInst *AbsF = Builder.CreateCall(II->getCalledFunction(), {FVal});
SelectInst *SI = SelectInst::Create(Cond, AbsT, AbsF);
diff --git a/llvm/test/Transforms/InstCombine/intrinsic-select.ll b/llvm/test/Transforms/InstCombine/intrinsic-select.ll
index 4ce2908a630785..a8117ce663a6d0 100644
--- a/llvm/test/Transforms/InstCombine/intrinsic-select.ll
+++ b/llvm/test/Transforms/InstCombine/intrinsic-select.ll
@@ -2,6 +2,7 @@
; RUN: opt -passes=instcombine -S < %s | FileCheck %s
declare void @use(i32)
+declare void @usef32(float)
declare i32 @llvm.ctlz.i32(i32, i1)
declare <3 x i17> @llvm.ctlz.v3i17(<3 x i17>, i1)
@@ -344,3 +345,29 @@ define double @test_fabs_select_fmf2(i1 %cond, double %a) {
%fabs = call nnan ninf nsz double @llvm.fabs.f64(double %sel1)
ret double %fabs
}
+
+define float @test_fabs_select_multiuse(i1 %cond, float %x) {
+; CHECK-LABEL: @test_fabs_select_multiuse(
+; CHECK-NEXT: [[SELECT:%.*]] = select i1 [[COND:%.*]], float [[X:%.*]], float 0x7FF0000000000000
+; CHECK-NEXT: call void @usef32(float [[SELECT]])
+; CHECK-NEXT: [[FABS:%.*]] = call float @llvm.fabs.f32(float [[SELECT]])
+; CHECK-NEXT: ret float [[FABS]]
+;
+ %select = select i1 %cond, float %x, float 0x7FF0000000000000
+ call void @usef32(float %select)
+ %fabs = call float @llvm.fabs.f32(float %select)
+ ret float %fabs
+}
+
+define float @test_fabs_select_multiuse_both_constant(i1 %cond, float %x) {
+; CHECK-LABEL: @test_fabs_select_multiuse_both_constant(
+; CHECK-NEXT: [[SELECT:%.*]] = select i1 [[COND:%.*]], float -1.000000e+00, float -2.000000e+00
+; CHECK-NEXT: call void @usef32(float [[SELECT]])
+; CHECK-NEXT: [[FABS:%.*]] = select i1 [[COND]], float 1.000000e+00, float 2.000000e+00
+; CHECK-NEXT: ret float [[FABS]]
+;
+ %select = select i1 %cond, float -1.0, float -2.0
+ call void @usef32(float %select)
+ %fabs = call float @llvm.fabs.f32(float %select)
+ ret float %fabs
+}
|
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/12661 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes multi-use issue introduced by #86390.
It also forbids the folding offabs (select Cond, TrueC, FalseC)when select has external uses.fabsis always cheaper than a select with constant arms.Update: allow this to fix a regression in ocio