[X86,SimplifyCFG] Use passthru to reduce select #108754
Merged
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
|
@llvm/pr-subscribers-llvm-transforms Author: Phoebe Wang (phoebewang) ChangesFull diff: https://github.com/llvm/llvm-project/pull/108754.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index f9db996cdc3583..5cebfbadf22069 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -3335,12 +3335,21 @@ bool SimplifyCFGOpt::speculativelyExecuteBB(BranchInst *BI,
assert(!getLoadStoreType(I)->isVectorTy() && "not implemented");
auto *Op0 = I->getOperand(0);
Instruction *MaskedLoadStore = nullptr;
+ PHINode *PN = nullptr;
if (auto *LI = dyn_cast<LoadInst>(I)) {
// Handle Load.
auto *Ty = I->getType();
- MaskedLoadStore = Builder.CreateMaskedLoad(FixedVectorType::get(Ty, 1),
- Op0, LI->getAlign(), Mask);
- I->replaceAllUsesWith(Builder.CreateBitCast(MaskedLoadStore, Ty));
+ Value *PassThru = nullptr;
+ if (I->hasOneUse())
+ if ((PN = dyn_cast<PHINode>(I->use_begin()->getUser())))
+ PassThru = Builder.CreateBitCast(PN->getIncomingValueForBlock(BB),
+ FixedVectorType::get(Ty, 1));
+ MaskedLoadStore = Builder.CreateMaskedLoad(
+ FixedVectorType::get(Ty, 1), Op0, LI->getAlign(), Mask, PassThru);
+ if (PN)
+ PN->replaceAllUsesWith(Builder.CreateBitCast(MaskedLoadStore, Ty));
+ else
+ I->replaceAllUsesWith(Builder.CreateBitCast(MaskedLoadStore, Ty));
} else {
// Handle Store.
auto *StoredVal =
@@ -3365,6 +3374,8 @@ bool SimplifyCFGOpt::speculativelyExecuteBB(BranchInst *BI,
return Node->getMetadataID() == Metadata::DIAssignIDKind;
});
MaskedLoadStore->copyMetadata(*I);
+ if (PN)
+ PN->eraseFromParent();
I->eraseFromParent();
}
diff --git a/llvm/test/Transforms/SimplifyCFG/X86/hoist-loads-stores-with-cf.ll b/llvm/test/Transforms/SimplifyCFG/X86/hoist-loads-stores-with-cf.ll
index 047ca717da8009..760334dc1d2815 100644
--- a/llvm/test/Transforms/SimplifyCFG/X86/hoist-loads-stores-with-cf.ll
+++ b/llvm/test/Transforms/SimplifyCFG/X86/hoist-loads-stores-with-cf.ll
@@ -72,10 +72,9 @@ define i32 @succ1to0_phi(ptr %p) {
; CHECK-NEXT: [[COND:%.*]] = icmp eq ptr [[P:%.*]], null
; CHECK-NEXT: [[TMP0:%.*]] = xor i1 [[COND]], true
; CHECK-NEXT: [[TMP1:%.*]] = bitcast i1 [[TMP0]] to <1 x i1>
-; CHECK-NEXT: [[TMP2:%.*]] = call <1 x i32> @llvm.masked.load.v1i32.p0(ptr [[P]], i32 4, <1 x i1> [[TMP1]], <1 x i32> poison)
+; CHECK-NEXT: [[TMP2:%.*]] = call <1 x i32> @llvm.masked.load.v1i32.p0(ptr [[P]], i32 4, <1 x i1> [[TMP1]], <1 x i32> zeroinitializer)
; CHECK-NEXT: [[TMP3:%.*]] = bitcast <1 x i32> [[TMP2]] to i32
-; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[COND]], i32 0, i32 [[TMP3]]
-; CHECK-NEXT: ret i32 [[SPEC_SELECT]]
+; CHECK-NEXT: ret i32 [[TMP3]]
;
entry:
%cond = icmp eq ptr %p, null
@@ -184,10 +183,9 @@ define i32 @load_from_gep(ptr %p) {
; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds i8, ptr [[P]], i64 16
; CHECK-NEXT: [[TMP0:%.*]] = xor i1 [[COND]], true
; CHECK-NEXT: [[TMP1:%.*]] = bitcast i1 [[TMP0]] to <1 x i1>
-; CHECK-NEXT: [[TMP2:%.*]] = call <1 x i32> @llvm.masked.load.v1i32.p0(ptr [[ARRAYIDX]], i32 4, <1 x i1> [[TMP1]], <1 x i32> poison)
+; CHECK-NEXT: [[TMP2:%.*]] = call <1 x i32> @llvm.masked.load.v1i32.p0(ptr [[ARRAYIDX]], i32 4, <1 x i1> [[TMP1]], <1 x i32> zeroinitializer)
; CHECK-NEXT: [[TMP3:%.*]] = bitcast <1 x i32> [[TMP2]] to i32
-; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[COND]], i32 0, i32 [[TMP3]]
-; CHECK-NEXT: ret i32 [[SPEC_SELECT]]
+; CHECK-NEXT: ret i32 [[TMP3]]
;
entry:
%cond = icmp eq ptr %p, null
|
dtcxzyw
reviewed
Sep 15, 2024
| Value *PassThru = nullptr; | ||
| if (I->hasOneUse()) | ||
| if ((PN = dyn_cast<PHINode>(I->use_begin()->getUser()))) | ||
| PassThru = Builder.CreateBitCast(PN->getIncomingValueForBlock(BB), |
There was a problem hiding this comment.
Can you please fix the header comment?
It should be /// %phi = phi [ %sub, %ThenBB ], [ 0, %BB ] .
| I->replaceAllUsesWith(Builder.CreateBitCast(MaskedLoadStore, Ty)); | ||
| Value *PassThru = nullptr; | ||
| if (I->hasOneUse()) | ||
| if ((PN = dyn_cast<PHINode>(I->use_begin()->getUser()))) |
There was a problem hiding this comment.
There was a problem hiding this comment.
Sorry, not get the question. Isn't it the suggested way?
dtcxzyw
requested changes
Sep 15, 2024
There was a problem hiding this comment.
; bin/opt -mtriple=x86_64 -mattr=+cf -passes='simplifycfg<hoist-loads-stores-with-cond-faulting>' reduced.ll -S
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-pc-linux-gnu"
define i32 @str_transcode0(i1 %0, ptr %1, i1 %.not135, ptr %p) {
rb_check_arity.exit:
br i1 %0, label %.thread, label %2
2: ; preds = %rb_check_arity.exit
br i1 %.not135, label %3, label %.thread
3: ; preds = %2
%4 = load i64, ptr %p, align 8
br label %.thread
.thread: ; preds = %3, %2, %rb_check_arity.exit
%5 = phi i64 [ %4, %3 ], [ 0, %2 ], [ 0, %rb_check_arity.exit ]
store i64 %5, ptr %1, align 8
ret i32 0
}
Instruction does not dominate all uses!
%5 = bitcast <1 x i64> %4 to i64
store i64 %5, ptr %1, align 8
LLVM ERROR: Broken module found, compilation aborted!
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0. Program arguments: bin/opt -mtriple=x86_64 -mattr=+cf -passes=simplifycfg<hoist-loads-stores-with-cond-faulting> reduced.ll -S
1. Running pass "verify" on module "reduced.ll"
#0 0x00007e6a03014742 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/dtcxzyw/WorkSpace/Projects/compilers/LLVM/llvm-build/bin/../lib/libLLVMSupport.so.20.0git+0x214742)
#1 0x00007e6a0301160f llvm::sys::RunSignalHandlers() (/home/dtcxzyw/WorkSpace/Projects/compilers/LLVM/llvm-build/bin/../lib/libLLVMSupport.so.20.0git+0x21160f)
#2 0x00007e6a03011755 SignalHandler(int) Signals.cpp:0:0
#3 0x00007e6a02a42520 (/lib/x86_64-linux-gnu/libc.so.6+0x42520)
#4 0x00007e6a02a969fc __pthread_kill_implementation ./nptl/pthread_kill.c:44:76
#5 0x00007e6a02a969fc __pthread_kill_internal ./nptl/pthread_kill.c:78:10
#6 0x00007e6a02a969fc pthread_kill ./nptl/pthread_kill.c:89:10
#7 0x00007e6a02a42476 gsignal ./signal/../sysdeps/posix/raise.c:27:6
#8 0x00007e6a02a287f3 abort ./stdlib/abort.c:81:7
#9 0x00007e6a02e6d70f llvm::report_fatal_error(llvm::Twine const&, bool) (.cold) ErrorHandling.cpp:0:0
#10 0x00007e6a02f1cc90 unsigned long std::uniform_int_distribution<unsigned long>::operator()<std::random_device>(std::random_device&, std::uniform_int_distribution<unsigned long>::param_type const&) (.isra.0) ExponentialBackoff.cpp:0:0
#11 0x00007e69fb996428 (/home/dtcxzyw/WorkSpace/Projects/compilers/LLVM/llvm-build/bin/../lib/../lib/libLLVMCore.so.20.0git+0x396428)
#12 0x00007e6a0323b645 llvm::detail::PassModel<llvm::Module, llvm::VerifierPass, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/home/dtcxzyw/WorkSpace/Projects/compilers/LLVM/llvm-build/bin/../lib/libLLVMOptDriver.so.20.0git+0x1f645)
#13 0x00007e69fb95386a llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/home/dtcxzyw/WorkSpace/Projects/compilers/LLVM/llvm-build/bin/../lib/../lib/libLLVMCore.so.20.0git+0x35386a)
#14 0x00007e6a032491b1 llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRef<llvm::PassPlugin>, llvm::ArrayRef<std::function<void (llvm::PassBuilder&)>>, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool, bool, bool) (/home/dtcxzyw/WorkSpace/Projects/compilers/LLVM/llvm-build/bin/../lib/libLLVMOptDriver.so.20.0git+0x2d1b1)
#15 0x00007e6a03254934 optMain (/home/dtcxzyw/WorkSpace/Projects/compilers/LLVM/llvm-build/bin/../lib/libLLVMOptDriver.so.20.0git+0x38934)
#16 0x00007e6a02a29d90 __libc_start_call_main ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#17 0x00007e6a02a29e40 call_init ./csu/../csu/libc-start.c:128:20
#18 0x00007e6a02a29e40 __libc_start_main ./csu/../csu/libc-start.c:379:5
#19 0x000060da02e96095 _start (bin/opt+0x1095)
Aborted (core dumped)
|
I prefer to do this simplification in InstCombine: 712b8f6. @nikic @KanRobert Any thoughts? |
Thanks for the test case! Fixed. |
I found it's not difficult to solve above crash. But I'm open to either way. |
dtcxzyw
approved these changes
Sep 16, 2024
RKSimon
approved these changes
Sep 16, 2024
KanRobert
reviewed
Sep 23, 2024
| @@ -674,6 +672,62 @@ if.false: | |||
| ret void | |||
| } | |||
|
|
|||
| define i32 @str_transcode0(i1 %cond1, ptr %p, i1 %cond2) { | |||
There was a problem hiding this comment.
Probably rename this function? I assume the function name should show what it's testing.
KanRobert
reviewed
Sep 23, 2024
| if ((PN = dyn_cast<PHINode>(U))) { | ||
| PassThru = Builder.CreateBitCast(PN->getIncomingValueForBlock(BB), | ||
| FixedVectorType::get(Ty, 1)); | ||
| break; |
There was a problem hiding this comment.
Should we add test for two PHI nodes?
phoebewang
added a commit
to phoebewang/llvm-project
that referenced
this pull request
Sep 23, 2024
Address post commit comments in llvm#108754.
phoebewang
added a commit
that referenced
this pull request
Sep 23, 2024
Address post commit comments in #108754.
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.
No description provided.