Skip to content

Commit d026c20

Browse files
farzonlfarzonl
committed
[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.
1 parent 9cb5c00 commit d026c20

File tree

3 files changed

+29
-25
lines changed

3 files changed

+29
-25
lines changed

‎llvm/lib/Target/DirectX/DXILLegalizePass.cpp

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ using namespace llvm;
2626

2727
static void legalizeFreeze(Instruction &I,
2828
SmallVectorImpl<Instruction *> &ToRemove,
29-
DenseMap<Value *, Value *>) {
29+
DenseMap<Value *, Value *>, bool &) {
3030
auto *FI = dyn_cast<FreezeInst>(&I);
3131
if (!FI)
3232
return;
@@ -37,7 +37,7 @@ static void legalizeFreeze(Instruction &I,
3737

3838
static void fixI8UseChain(Instruction &I,
3939
SmallVectorImpl<Instruction *> &ToRemove,
40-
DenseMap<Value *, Value *> &ReplacedValues) {
40+
DenseMap<Value *, Value *> &ReplacedValues, bool &) {
4141

4242
auto ProcessOperands = [&](SmallVector<Value *> &NewOperands) {
4343
Type *InstrType = IntegerType::get(I.getContext(), 32);
@@ -253,7 +253,8 @@ static void fixI8UseChain(Instruction &I,
253253

254254
static void upcastI8AllocasAndUses(Instruction &I,
255255
SmallVectorImpl<Instruction *> &ToRemove,
256-
DenseMap<Value *, Value *> &ReplacedValues) {
256+
DenseMap<Value *, Value *> &ReplacedValues,
257+
bool &) {
257258
auto *AI = dyn_cast<AllocaInst>(&I);
258259
if (!AI || !AI->getAllocatedType()->isIntegerTy(8))
259260
return;
@@ -303,7 +304,7 @@ static void upcastI8AllocasAndUses(Instruction &I,
303304
static void
304305
downcastI64toI32InsertExtractElements(Instruction &I,
305306
SmallVectorImpl<Instruction *> &ToRemove,
306-
DenseMap<Value *, Value *> &) {
307+
DenseMap<Value *, Value *> &, bool &) {
307308

308309
if (auto *Extract = dyn_cast<ExtractElementInst>(&I)) {
309310
Value *Idx = Extract->getIndexOperand();
@@ -455,7 +456,7 @@ static void emitMemsetExpansion(IRBuilder<> &Builder, Value *Dst, Value *Val,
455456
// vector. `ReplacedValues` is unused.
456457
static void legalizeMemCpy(Instruction &I,
457458
SmallVectorImpl<Instruction *> &ToRemove,
458-
DenseMap<Value *, Value *> &ReplacedValues) {
459+
DenseMap<Value *, Value *> &ReplacedValues, bool &) {
459460

460461
CallInst *CI = dyn_cast<CallInst>(&I);
461462
if (!CI)
@@ -480,7 +481,7 @@ static void legalizeMemCpy(Instruction &I,
480481

481482
static void legalizeMemSet(Instruction &I,
482483
SmallVectorImpl<Instruction *> &ToRemove,
483-
DenseMap<Value *, Value *> &ReplacedValues) {
484+
DenseMap<Value *, Value *> &ReplacedValues, bool &) {
484485

485486
CallInst *CI = dyn_cast<CallInst>(&I);
486487
if (!CI)
@@ -501,7 +502,7 @@ static void legalizeMemSet(Instruction &I,
501502

502503
static void updateFnegToFsub(Instruction &I,
503504
SmallVectorImpl<Instruction *> &ToRemove,
504-
DenseMap<Value *, Value *> &) {
505+
DenseMap<Value *, Value *> &, bool &) {
505506
const Intrinsic::ID ID = I.getOpcode();
506507
if (ID != Instruction::FNeg)
507508
return;
@@ -516,7 +517,7 @@ static void updateFnegToFsub(Instruction &I,
516517
static void
517518
legalizeGetHighLowi64Bytes(Instruction &I,
518519
SmallVectorImpl<Instruction *> &ToRemove,
519-
DenseMap<Value *, Value *> &ReplacedValues) {
520+
DenseMap<Value *, Value *> &ReplacedValues, bool &) {
520521
if (auto *BitCast = dyn_cast<BitCastInst>(&I)) {
521522
if (BitCast->getDestTy() ==
522523
FixedVectorType::get(Type::getInt32Ty(I.getContext()), 2) &&
@@ -562,10 +563,9 @@ legalizeGetHighLowi64Bytes(Instruction &I,
562563
}
563564
}
564565

565-
static void
566-
legalizeScalarLoadStoreOnArrays(Instruction &I,
567-
SmallVectorImpl<Instruction *> &ToRemove,
568-
DenseMap<Value *, Value *> &) {
566+
static void legalizeScalarLoadStoreOnArrays(
567+
Instruction &I, SmallVectorImpl<Instruction *> &ToRemove,
568+
DenseMap<Value *, Value *> &, bool &MadeChange) {
569569

570570
Value *PtrOp;
571571
unsigned PtrOpIndex;
@@ -607,6 +607,7 @@ legalizeScalarLoadStoreOnArrays(Instruction &I,
607607
Value *GEP = GetElementPtrInst::Create(
608608
ArrayTy, PtrOp, {Zero, Zero}, GEPNoWrapFlags::all(), "", I.getIterator());
609609
I.setOperand(PtrOpIndex, GEP);
610+
MadeChange = true;
610611
}
611612

612613
namespace {
@@ -623,13 +624,15 @@ class DXILLegalizationPipeline {
623624
ToRemove.clear();
624625
ReplacedValues.clear();
625626
for (auto &I : instructions(F)) {
626-
for (auto &LegalizationFn : LegalizationPipeline[Stage])
627-
LegalizationFn(I, ToRemove, ReplacedValues);
627+
for (auto &LegalizationFn : LegalizationPipeline[Stage]) {
628+
bool PerLegalizationChange = false;
629+
LegalizationFn(I, ToRemove, ReplacedValues, PerLegalizationChange);
630+
MadeChange |= PerLegalizationChange;
631+
}
628632
}
629633

630634
for (auto *Inst : reverse(ToRemove))
631635
Inst->eraseFromParent();
632-
633636
MadeChange |= !ToRemove.empty();
634637
}
635638
return MadeChange;
@@ -640,7 +643,7 @@ class DXILLegalizationPipeline {
640643

641644
using LegalizationFnTy =
642645
std::function<void(Instruction &, SmallVectorImpl<Instruction *> &,
643-
DenseMap<Value *, Value *> &)>;
646+
DenseMap<Value *, Value *> &, bool &)>;
644647

645648
SmallVector<LegalizationFnTy> LegalizationPipeline[NumStages];
646649

@@ -676,16 +679,16 @@ class DXILLegalizeLegacy : public FunctionPass {
676679
PreservedAnalyses DXILLegalizePass::run(Function &F,
677680
FunctionAnalysisManager &FAM) {
678681
DXILLegalizationPipeline DXLegalize;
679-
bool MadeChanges = DXLegalize.runLegalizationPipeline(F);
680-
if (!MadeChanges)
681-
return PreservedAnalyses::all();
682-
PreservedAnalyses PA;
683-
return PA;
682+
DXLegalize.runLegalizationPipeline(F);
683+
// Note: Nothing seems to need Legalization to preserve analysis.
684+
// Simplest fix was to not preserve analysis.
685+
return PreservedAnalyses::none();
684686
}
685687

686688
bool DXILLegalizeLegacy::runOnFunction(Function &F) {
687689
DXILLegalizationPipeline DXLegalize;
688-
return DXLegalize.runLegalizationPipeline(F);
690+
DXLegalize.runLegalizationPipeline(F);
691+
return true;
689692
}
690693

691694
char DXILLegalizeLegacy::ID = 0;

‎llvm/lib/Target/DirectX/DXILResourceAccess.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,6 @@ static void replaceAccess(IntrinsicInst *II, dxil::ResourceTypeInfo &RTI) {
241241
}
242242

243243
static bool transformResourcePointers(Function &F, DXILResourceTypeMap &DRTM) {
244-
bool Changed = false;
245244
SmallVector<std::pair<IntrinsicInst *, dxil::ResourceTypeInfo>> Resources;
246245
for (BasicBlock &BB : F)
247246
for (Instruction &I : BB)
@@ -254,7 +253,7 @@ static bool transformResourcePointers(Function &F, DXILResourceTypeMap &DRTM) {
254253
for (auto &[II, RI] : Resources)
255254
replaceAccess(II, RI);
256255

257-
return Changed;
256+
return !Resources.empty();
258257
}
259258

260259
PreservedAnalyses DXILResourceAccess::run(Function &F,

‎llvm/lib/Transforms/Scalar/Scalarizer.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,8 +458,10 @@ bool ScalarizerVisitor::visit(Function &F) {
458458
Instruction *I = &*II;
459459
bool Done = InstVisitor::visit(I);
460460
++II;
461-
if (Done && I->getType()->isVoidTy())
461+
if (Done && I->getType()->isVoidTy()) {
462462
I->eraseFromParent();
463+
Scalarized = true;
464+
}
463465
}
464466
}
465467
return finish();

0 commit comments

Comments
 (0)