[llvm] [DirectX] Fix crash in passes when building with LLVM_ENABLE_EXPENSIVE_CHECKS (PR #150483)
Farzon Lotfi via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 24 12:06:38 PDT 2025
https://github.com/farzonl updated https://github.com/llvm/llvm-project/pull/150483
>From f73c390db50d171bce00a9b1d8ed8a4c01ff5f3d Mon Sep 17 00:00:00 2001
From: Farzon Lotfi <farzonlotfi at microsoft.com>
Date: Thu, 24 Jul 2025 13:59:28 -0400
Subject: [PATCH] [DirectX] fix crash in passes when building with
LLVM_ENABLE_EXPENSIVE_CHECKS
fixes #148681
For the scalarizer pass we just need to indicate that scalarization took
place, I used the logic for knowing when to eraseFromParent to indicate
this.
For the DXILLegalizePass the new `legalizeScalarLoadStoreOnArrays` did
not use `ToRemove` which means our uses of !ToRemove.empty(); was no
longer correct. This meant each legalization now needed a means of
indicated if a change was maded.
For DXILResourceAccess.cpp the `Changed` bool was never set to true.
So removed it and replaced it with `!Resources.empty();` since we only
call `replaceAccess` if we have items in Resources.
---
llvm/lib/Target/DirectX/DXILLegalizePass.cpp | 34 +++++++++++--------
.../lib/Target/DirectX/DXILResourceAccess.cpp | 3 +-
llvm/lib/Transforms/Scalar/Scalarizer.cpp | 4 ++-
3 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/llvm/lib/Target/DirectX/DXILLegalizePass.cpp b/llvm/lib/Target/DirectX/DXILLegalizePass.cpp
index c73648f21e8d7..c1cbff6b58be8 100644
--- a/llvm/lib/Target/DirectX/DXILLegalizePass.cpp
+++ b/llvm/lib/Target/DirectX/DXILLegalizePass.cpp
@@ -26,7 +26,7 @@ using namespace llvm;
static void legalizeFreeze(Instruction &I,
SmallVectorImpl<Instruction *> &ToRemove,
- DenseMap<Value *, Value *>) {
+ DenseMap<Value *, Value *>, bool &) {
auto *FI = dyn_cast<FreezeInst>(&I);
if (!FI)
return;
@@ -37,7 +37,7 @@ static void legalizeFreeze(Instruction &I,
static void fixI8UseChain(Instruction &I,
SmallVectorImpl<Instruction *> &ToRemove,
- DenseMap<Value *, Value *> &ReplacedValues) {
+ DenseMap<Value *, Value *> &ReplacedValues, bool &) {
auto ProcessOperands = [&](SmallVector<Value *> &NewOperands) {
Type *InstrType = IntegerType::get(I.getContext(), 32);
@@ -253,7 +253,8 @@ static void fixI8UseChain(Instruction &I,
static void upcastI8AllocasAndUses(Instruction &I,
SmallVectorImpl<Instruction *> &ToRemove,
- DenseMap<Value *, Value *> &ReplacedValues) {
+ DenseMap<Value *, Value *> &ReplacedValues,
+ bool &) {
auto *AI = dyn_cast<AllocaInst>(&I);
if (!AI || !AI->getAllocatedType()->isIntegerTy(8))
return;
@@ -303,7 +304,7 @@ static void upcastI8AllocasAndUses(Instruction &I,
static void
downcastI64toI32InsertExtractElements(Instruction &I,
SmallVectorImpl<Instruction *> &ToRemove,
- DenseMap<Value *, Value *> &) {
+ DenseMap<Value *, Value *> &, bool &) {
if (auto *Extract = dyn_cast<ExtractElementInst>(&I)) {
Value *Idx = Extract->getIndexOperand();
@@ -455,7 +456,7 @@ static void emitMemsetExpansion(IRBuilder<> &Builder, Value *Dst, Value *Val,
// vector. `ReplacedValues` is unused.
static void legalizeMemCpy(Instruction &I,
SmallVectorImpl<Instruction *> &ToRemove,
- DenseMap<Value *, Value *> &ReplacedValues) {
+ DenseMap<Value *, Value *> &ReplacedValues, bool &) {
CallInst *CI = dyn_cast<CallInst>(&I);
if (!CI)
@@ -480,7 +481,7 @@ static void legalizeMemCpy(Instruction &I,
static void legalizeMemSet(Instruction &I,
SmallVectorImpl<Instruction *> &ToRemove,
- DenseMap<Value *, Value *> &ReplacedValues) {
+ DenseMap<Value *, Value *> &ReplacedValues, bool &) {
CallInst *CI = dyn_cast<CallInst>(&I);
if (!CI)
@@ -501,7 +502,7 @@ static void legalizeMemSet(Instruction &I,
static void updateFnegToFsub(Instruction &I,
SmallVectorImpl<Instruction *> &ToRemove,
- DenseMap<Value *, Value *> &) {
+ DenseMap<Value *, Value *> &, bool &) {
const Intrinsic::ID ID = I.getOpcode();
if (ID != Instruction::FNeg)
return;
@@ -516,7 +517,7 @@ static void updateFnegToFsub(Instruction &I,
static void
legalizeGetHighLowi64Bytes(Instruction &I,
SmallVectorImpl<Instruction *> &ToRemove,
- DenseMap<Value *, Value *> &ReplacedValues) {
+ DenseMap<Value *, Value *> &ReplacedValues, bool &) {
if (auto *BitCast = dyn_cast<BitCastInst>(&I)) {
if (BitCast->getDestTy() ==
FixedVectorType::get(Type::getInt32Ty(I.getContext()), 2) &&
@@ -562,10 +563,9 @@ legalizeGetHighLowi64Bytes(Instruction &I,
}
}
-static void
-legalizeScalarLoadStoreOnArrays(Instruction &I,
- SmallVectorImpl<Instruction *> &ToRemove,
- DenseMap<Value *, Value *> &) {
+static void legalizeScalarLoadStoreOnArrays(
+ Instruction &I, SmallVectorImpl<Instruction *> &ToRemove,
+ DenseMap<Value *, Value *> &, bool &MadeChange) {
Value *PtrOp;
unsigned PtrOpIndex;
@@ -607,6 +607,7 @@ legalizeScalarLoadStoreOnArrays(Instruction &I,
Value *GEP = GetElementPtrInst::Create(
ArrayTy, PtrOp, {Zero, Zero}, GEPNoWrapFlags::all(), "", I.getIterator());
I.setOperand(PtrOpIndex, GEP);
+ MadeChange = true;
}
namespace {
@@ -623,8 +624,11 @@ class DXILLegalizationPipeline {
ToRemove.clear();
ReplacedValues.clear();
for (auto &I : instructions(F)) {
- for (auto &LegalizationFn : LegalizationPipeline[Stage])
- LegalizationFn(I, ToRemove, ReplacedValues);
+ for (auto &LegalizationFn : LegalizationPipeline[Stage]) {
+ bool PerLegalizationChange = false;
+ LegalizationFn(I, ToRemove, ReplacedValues, PerLegalizationChange);
+ MadeChange |= PerLegalizationChange;
+ }
}
for (auto *Inst : reverse(ToRemove))
@@ -640,7 +644,7 @@ class DXILLegalizationPipeline {
using LegalizationFnTy =
std::function<void(Instruction &, SmallVectorImpl<Instruction *> &,
- DenseMap<Value *, Value *> &)>;
+ DenseMap<Value *, Value *> &, bool &)>;
SmallVector<LegalizationFnTy> LegalizationPipeline[NumStages];
diff --git a/llvm/lib/Target/DirectX/DXILResourceAccess.cpp b/llvm/lib/Target/DirectX/DXILResourceAccess.cpp
index 566f3a98457a4..c33ec0efd73ca 100644
--- a/llvm/lib/Target/DirectX/DXILResourceAccess.cpp
+++ b/llvm/lib/Target/DirectX/DXILResourceAccess.cpp
@@ -241,7 +241,6 @@ static void replaceAccess(IntrinsicInst *II, dxil::ResourceTypeInfo &RTI) {
}
static bool transformResourcePointers(Function &F, DXILResourceTypeMap &DRTM) {
- bool Changed = false;
SmallVector<std::pair<IntrinsicInst *, dxil::ResourceTypeInfo>> Resources;
for (BasicBlock &BB : F)
for (Instruction &I : BB)
@@ -254,7 +253,7 @@ static bool transformResourcePointers(Function &F, DXILResourceTypeMap &DRTM) {
for (auto &[II, RI] : Resources)
replaceAccess(II, RI);
- return Changed;
+ return !Resources.empty();
}
PreservedAnalyses DXILResourceAccess::run(Function &F,
diff --git a/llvm/lib/Transforms/Scalar/Scalarizer.cpp b/llvm/lib/Transforms/Scalar/Scalarizer.cpp
index ced61cb8e51fe..aae5d6074ae8e 100644
--- a/llvm/lib/Transforms/Scalar/Scalarizer.cpp
+++ b/llvm/lib/Transforms/Scalar/Scalarizer.cpp
@@ -458,8 +458,10 @@ bool ScalarizerVisitor::visit(Function &F) {
Instruction *I = &*II;
bool Done = InstVisitor::visit(I);
++II;
- if (Done && I->getType()->isVoidTy())
+ if (Done && I->getType()->isVoidTy()) {
I->eraseFromParent();
+ Scalarized = true;
+ }
}
}
return finish();
More information about the llvm-commits
mailing list