[llvm] [DirectX] Bug fix for Data Scalarization crash (PR #118426)

Farzon Lotfi via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 16 12:03:38 PST 2024


================
@@ -106,6 +90,21 @@ bool DataScalarizerVisitor::visitLoadInst(LoadInst &LI) {
   unsigned NumOperands = LI.getNumOperands();
   for (unsigned I = 0; I < NumOperands; ++I) {
     Value *CurrOpperand = LI.getOperand(I);
+    ConstantExpr *CE = dyn_cast<ConstantExpr>(CurrOpperand);
+    if (CE && CE->getOpcode() == Instruction::GetElementPtr) {
+      GetElementPtrInst *OldGEP =
+          cast<GetElementPtrInst>(CE->getAsInstruction());
+      OldGEP->insertBefore(&LI);
+      Value *NewGEP = createNewGetElementPtr(*OldGEP);
----------------
farzonl wrote:

Few things going on here.  instead of calling `convertUsersOfConstantsToInstructions` we are going to do something simpler to split the GEP Operand into an instruction.

When you call `getAsInstruction()` off a ConstExpr it creates a new instruction. So that we don't lose track of it and so it gets parented we do `insertBefore`.

`createNewGetElementPtr` wasn't strictly necessary. We could have called `visitGetElementPtrInst` at the end and then we wouldn't need to do `OldGEP->eraseFromParent();` 

Infact calling `visitGetElementPtrInst` is how I'm doing it in `DXILFlattenArrays.cpp` because that pass has to deal with more complicated things to update the GEP chains. Only reason I decided to turn `visitGetElementPtrInst` into a helper is because of how straight forward it was to only operate on one instruction.  I don't have strong feelings on this approach so if you think of something better please let me know.


https://github.com/llvm/llvm-project/pull/118426


More information about the llvm-commits mailing list