Skip to content

[DirectX] Fix crash in passes when building with LLVM_ENABLE_EXPENSIVE_CHECKS #150483

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 24, 2025
+58 −46
Merged

[DirectX] Fix crash in passes when building with LLVM_ENABLE_EXPENSIVE_CHECKS #150483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f73c390
[DirectX] fix crash in passes when building with LLVM_ENABLE_EXPENSIV…
farzonl Jul 24, 2025
a293aa6
Make DXILLegalizePass not dependent on ToRemove for change status
farzonl Jul 24, 2025
5ad6b43
fix formatting
farzonl Jul 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 54 additions & 43 deletions llvm/lib/Target/DirectX/DXILLegalizePass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,19 @@

using namespace llvm;

static void legalizeFreeze(Instruction &I,
static bool legalizeFreeze(Instruction &I,
SmallVectorImpl<Instruction *> &ToRemove,
DenseMap<Value *, Value *>) {
auto *FI = dyn_cast<FreezeInst>(&I);
if (!FI)
return;
return false;

FI->replaceAllUsesWith(FI->getOperand(0));
ToRemove.push_back(FI);
return true;
}

static void fixI8UseChain(Instruction &I,
static bool fixI8UseChain(Instruction &I,
SmallVectorImpl<Instruction *> &ToRemove,
DenseMap<Value *, Value *> &ReplacedValues) {

Expand Down Expand Up @@ -74,19 +75,19 @@ static void fixI8UseChain(Instruction &I,
if (Trunc->getDestTy()->isIntegerTy(8)) {
ReplacedValues[Trunc] = Trunc->getOperand(0);
ToRemove.push_back(Trunc);
return;
return true;
}
}

if (auto *Store = dyn_cast<StoreInst>(&I)) {
if (!Store->getValueOperand()->getType()->isIntegerTy(8))
return;
return false;
SmallVector<Value *> NewOperands;
ProcessOperands(NewOperands);
Value *NewStore = Builder.CreateStore(NewOperands[0], NewOperands[1]);
ReplacedValues[Store] = NewStore;
ToRemove.push_back(Store);
return;
return true;
}

if (auto *Load = dyn_cast<LoadInst>(&I);
Expand All @@ -104,17 +105,17 @@ static void fixI8UseChain(Instruction &I,
LoadInst *NewLoad = Builder.CreateLoad(ElementType, NewOperands[0]);
ReplacedValues[Load] = NewLoad;
ToRemove.push_back(Load);
return;
return true;
}

if (auto *Load = dyn_cast<LoadInst>(&I);
Load && isa<ConstantExpr>(Load->getPointerOperand())) {
auto *CE = dyn_cast<ConstantExpr>(Load->getPointerOperand());
if (!(CE->getOpcode() == Instruction::GetElementPtr))
return;
return false;
auto *GEP = dyn_cast<GEPOperator>(CE);
if (!GEP->getSourceElementType()->isIntegerTy(8))
return;
return false;

Type *ElementType = Load->getType();
ConstantInt *Offset = dyn_cast<ConstantInt>(GEP->getOperand(1));
Expand Down Expand Up @@ -143,12 +144,12 @@ static void fixI8UseChain(Instruction &I,
ReplacedValues[Load] = NewLoad;
Load->replaceAllUsesWith(NewLoad);
ToRemove.push_back(Load);
return;
return true;
}

if (auto *BO = dyn_cast<BinaryOperator>(&I)) {
if (!I.getType()->isIntegerTy(8))
return;
return false;
SmallVector<Value *> NewOperands;
ProcessOperands(NewOperands);
Value *NewInst =
Expand All @@ -162,43 +163,43 @@ static void fixI8UseChain(Instruction &I,
}
ReplacedValues[BO] = NewInst;
ToRemove.push_back(BO);
return;
return true;
}

if (auto *Sel = dyn_cast<SelectInst>(&I)) {
if (!I.getType()->isIntegerTy(8))
return;
return false;
SmallVector<Value *> NewOperands;
ProcessOperands(NewOperands);
Value *NewInst = Builder.CreateSelect(Sel->getCondition(), NewOperands[1],
NewOperands[2]);
ReplacedValues[Sel] = NewInst;
ToRemove.push_back(Sel);
return;
return true;
}

if (auto *Cmp = dyn_cast<CmpInst>(&I)) {
if (!Cmp->getOperand(0)->getType()->isIntegerTy(8))
return;
return false;
SmallVector<Value *> NewOperands;
ProcessOperands(NewOperands);
Value *NewInst =
Builder.CreateCmp(Cmp->getPredicate(), NewOperands[0], NewOperands[1]);
Cmp->replaceAllUsesWith(NewInst);
ReplacedValues[Cmp] = NewInst;
ToRemove.push_back(Cmp);
return;
return true;
}

if (auto *Cast = dyn_cast<CastInst>(&I)) {
if (!Cast->getSrcTy()->isIntegerTy(8))
return;
return false;

ToRemove.push_back(Cast);
auto *Replacement = ReplacedValues[Cast->getOperand(0)];
if (Cast->getType() == Replacement->getType()) {
Cast->replaceAllUsesWith(Replacement);
return;
return true;
}

Value *AdjustedCast = nullptr;
Expand All @@ -213,7 +214,7 @@ static void fixI8UseChain(Instruction &I,
if (auto *GEP = dyn_cast<GetElementPtrInst>(&I)) {
if (!GEP->getType()->isPointerTy() ||
!GEP->getSourceElementType()->isIntegerTy(8))
return;
return false;

Value *BasePtr = GEP->getPointerOperand();
if (ReplacedValues.count(BasePtr))
Expand Down Expand Up @@ -248,15 +249,17 @@ static void fixI8UseChain(Instruction &I,
ReplacedValues[GEP] = NewGEP;
GEP->replaceAllUsesWith(NewGEP);
ToRemove.push_back(GEP);
return true;
}
return false;
}

static void upcastI8AllocasAndUses(Instruction &I,
static bool upcastI8AllocasAndUses(Instruction &I,
SmallVectorImpl<Instruction *> &ToRemove,
DenseMap<Value *, Value *> &ReplacedValues) {
auto *AI = dyn_cast<AllocaInst>(&I);
if (!AI || !AI->getAllocatedType()->isIntegerTy(8))
return;
return false;

Type *SmallestType = nullptr;

Expand Down Expand Up @@ -291,16 +294,17 @@ static void upcastI8AllocasAndUses(Instruction &I,
}

if (!SmallestType)
return; // no valid casts found
return false; // no valid casts found

// Replace alloca
IRBuilder<> Builder(AI);
auto *NewAlloca = Builder.CreateAlloca(SmallestType);
ReplacedValues[AI] = NewAlloca;
ToRemove.push_back(AI);
return true;
}

static void
static bool
downcastI64toI32InsertExtractElements(Instruction &I,
SmallVectorImpl<Instruction *> &ToRemove,
DenseMap<Value *, Value *> &) {
Expand All @@ -318,6 +322,7 @@ downcastI64toI32InsertExtractElements(Instruction &I,

Extract->replaceAllUsesWith(NewExtract);
ToRemove.push_back(Extract);
return true;
}
}

Expand All @@ -335,8 +340,10 @@ downcastI64toI32InsertExtractElements(Instruction &I,

Insert->replaceAllUsesWith(Insert32Index);
ToRemove.push_back(Insert);
return true;
}
}
return false;
}

static void emitMemcpyExpansion(IRBuilder<> &Builder, Value *Dst, Value *Src,
Expand Down Expand Up @@ -453,17 +460,17 @@ static void emitMemsetExpansion(IRBuilder<> &Builder, Value *Dst, Value *Val,
// Expands the instruction `I` into corresponding loads and stores if it is a
// memcpy call. In that case, the call instruction is added to the `ToRemove`
// vector. `ReplacedValues` is unused.
static void legalizeMemCpy(Instruction &I,
static bool legalizeMemCpy(Instruction &I,
SmallVectorImpl<Instruction *> &ToRemove,
DenseMap<Value *, Value *> &ReplacedValues) {

CallInst *CI = dyn_cast<CallInst>(&I);
if (!CI)
return;
return false;

Intrinsic::ID ID = CI->getIntrinsicID();
if (ID != Intrinsic::memcpy)
return;
return false;

IRBuilder<> Builder(&I);
Value *Dst = CI->getArgOperand(0);
Expand All @@ -476,19 +483,20 @@ static void legalizeMemCpy(Instruction &I,
assert(IsVolatile->getZExtValue() == 0 && "Expected IsVolatile to be false");
emitMemcpyExpansion(Builder, Dst, Src, Length);
ToRemove.push_back(CI);
return true;
}

static void legalizeMemSet(Instruction &I,
static bool legalizeMemSet(Instruction &I,
SmallVectorImpl<Instruction *> &ToRemove,
DenseMap<Value *, Value *> &ReplacedValues) {

CallInst *CI = dyn_cast<CallInst>(&I);
if (!CI)
return;
return false;

Intrinsic::ID ID = CI->getIntrinsicID();
if (ID != Intrinsic::memset)
return;
return false;

IRBuilder<> Builder(&I);
Value *Dst = CI->getArgOperand(0);
Expand All @@ -497,23 +505,25 @@ static void legalizeMemSet(Instruction &I,
assert(Size && "Expected Size to be a ConstantInt");
emitMemsetExpansion(Builder, Dst, Val, Size, ReplacedValues);
ToRemove.push_back(CI);
return true;
}

static void updateFnegToFsub(Instruction &I,
static bool updateFnegToFsub(Instruction &I,
SmallVectorImpl<Instruction *> &ToRemove,
DenseMap<Value *, Value *> &) {
const Intrinsic::ID ID = I.getOpcode();
if (ID != Instruction::FNeg)
return;
return false;

IRBuilder<> Builder(&I);
Value *In = I.getOperand(0);
Value *Zero = ConstantFP::get(In->getType(), -0.0);
I.replaceAllUsesWith(Builder.CreateFSub(Zero, In));
ToRemove.push_back(&I);
return true;
}

static void
static bool
legalizeGetHighLowi64Bytes(Instruction &I,
SmallVectorImpl<Instruction *> &ToRemove,
DenseMap<Value *, Value *> &ReplacedValues) {
Expand All @@ -523,13 +533,13 @@ legalizeGetHighLowi64Bytes(Instruction &I,
BitCast->getSrcTy()->isIntegerTy(64)) {
ToRemove.push_back(BitCast);
ReplacedValues[BitCast] = BitCast->getOperand(0);
return;
return true;
}
}

if (auto *Extract = dyn_cast<ExtractElementInst>(&I)) {
if (!dyn_cast<BitCastInst>(Extract->getVectorOperand()))
return;
return false;
auto *VecTy = dyn_cast<FixedVectorType>(Extract->getVectorOperandType());
if (VecTy && VecTy->getElementType()->isIntegerTy(32) &&
VecTy->getNumElements() == 2) {
Expand Down Expand Up @@ -557,12 +567,14 @@ legalizeGetHighLowi64Bytes(Instruction &I,
}
ToRemove.push_back(Extract);
Extract->replaceAllUsesWith(ReplacedValues[Extract]);
return true;
}
}
}
return false;
}

static void
static bool
legalizeScalarLoadStoreOnArrays(Instruction &I,
SmallVectorImpl<Instruction *> &ToRemove,
DenseMap<Value *, Value *> &) {
Expand All @@ -579,25 +591,25 @@ legalizeScalarLoadStoreOnArrays(Instruction &I,
PtrOpIndex = SI->getPointerOperandIndex();
LoadStoreTy = SI->getValueOperand()->getType();
} else
return;
return false;

// If the load/store is not of a single-value type (i.e., scalar or vector)
// then we do not modify it. It shouldn't be a vector either because the
// dxil-data-scalarization pass is expected to run before this, but it's not
// incorrect to apply this transformation to vector load/stores.
if (!LoadStoreTy->isSingleValueType())
return;
return false;

Type *ArrayTy;
if (auto *GlobalVarPtrOp = dyn_cast<GlobalVariable>(PtrOp))
ArrayTy = GlobalVarPtrOp->getValueType();
else if (auto *AllocaPtrOp = dyn_cast<AllocaInst>(PtrOp))
ArrayTy = AllocaPtrOp->getAllocatedType();
else
return;
return false;

if (!isa<ArrayType>(ArrayTy))
return;
return false;

assert(ArrayTy->getArrayElementType() == LoadStoreTy &&
"Expected array element type to be the same as to the scalar load or "
Expand All @@ -607,6 +619,7 @@ legalizeScalarLoadStoreOnArrays(Instruction &I,
Value *GEP = GetElementPtrInst::Create(
ArrayTy, PtrOp, {Zero, Zero}, GEPNoWrapFlags::all(), "", I.getIterator());
I.setOperand(PtrOpIndex, GEP);
return true;
}

namespace {
Expand All @@ -624,13 +637,11 @@ class DXILLegalizationPipeline {
ReplacedValues.clear();
for (auto &I : instructions(F)) {
for (auto &LegalizationFn : LegalizationPipeline[Stage])
LegalizationFn(I, ToRemove, ReplacedValues);
MadeChange |= LegalizationFn(I, ToRemove, ReplacedValues);
}

for (auto *Inst : reverse(ToRemove))
Inst->eraseFromParent();

MadeChange |= !ToRemove.empty();
}
return MadeChange;
}
Expand All @@ -639,7 +650,7 @@ class DXILLegalizationPipeline {
enum LegalizationStage { Stage1 = 0, Stage2 = 1, NumStages };

using LegalizationFnTy =
std::function<void(Instruction &, SmallVectorImpl<Instruction *> &,
std::function<bool(Instruction &, SmallVectorImpl<Instruction *> &,
DenseMap<Value *, Value *> &)>;

SmallVector<LegalizationFnTy> LegalizationPipeline[NumStages];
Expand Down
3 changes: 1 addition & 2 deletions llvm/lib/Target/DirectX/DXILResourceAccess.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion llvm/lib/Transforms/Scalar/Scalarizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down