Issue |
145924
|
Summary |
[DirectX] Scalarizer is producing GEP chains that fail validation
|
Labels |
bug
|
Assignees |
|
Reporter |
Icohedron
|
The validator doesn't like GEP chains where an array GEP is followed by a scalar GEP.
If you compile the following LLVM IR to DXIL using `llc -filetype=obj -mtriple=dxil-pc-shadermodel6.3-library`
```llvm
define i32 @gep_chain() #0 {
%1 = alloca [4 x i32], align 4
%2 = getelementptr inbounds nuw [4 x i32], ptr %1, i32 0, i32 2
%3 = getelementptr inbounds nuw i32, ptr %2, i32 1
%4 = load i32, ptr %3
ret i32 %4
}
attributes #0 = { convergent norecurse nounwind "hlsl.export"}
```
and then validate the resulting dxil using dxv, you get the following validation error (#140417):
```
Function: gep_chain: error: Access to out-of-bounds memory is disallowed.
note: at '%3 = getelementptr inbounds i32, i32* %2, i32 1' in block '#0' of function 'gep_chain'.
Validation failed.
```
These GEP chains can appear when compiling HLSL shaders such as this one: https://godbolt.org/z/ExeGKb7MG
The scalarizer pass is responsible for emitting the scalar GEP in this case, which itself occurs after the dxil-flatten-arrays pass that had attempted to collapse GEP chains. Therefore, we need a later pass, such as the dxil-legalize pass, or a new pass to collapse the GEP chains again.
I propose that to solve this cleanly, we should shift the responsibility of collapsing GEP chains from the dxil-flatten-arrays pass over to the dxil-legalize pass.
Another reason to move GEP-chain-collapsing over to the dxil-legalize pass is to consolidate i8 GEP transformations as well. Currently i8 GEPs can appear during dxil-data-scalarization and dxil-flatten-arrays (see #145780), which occurs before the dxil-legalize pass that is responsible for i8 GEP legalization. By moving the GEP-chain-collapsing logic from the dxil-flatten-arrays pass over to the dxil-legalize pass, the dxil-legalize pass can simultaneously collapse GEP chains and legalize any i8 and scalar GEPs in those GEP chains.
The dxil-flatten-arrays pass' visitGetElementPtrInst can be simplified down to the following:
```c++
/// This visitor simply converts GEPs on multidimensional arrays into GEPs on
/// flattened arrays. A later "GEP chain collapser" pass will be used to combine
/// GEP chains into single flat GEPs, including cases of i8 GEPs, and scalar
/// GEPs introduced by the scalarizer.
bool DXILFlattenArraysVisitor::visitGetElementPtrInst(GetElementPtrInst &GEP) {
if (!isMultiDimensionalArray(GEP.getSourceElementType())) {
return false;
}
ArrayType *ArrType = cast<ArrayType>(GEP.getSourceElementType());
IRBuilder<> Builder(&GEP);
auto [TotalElements, BaseType] = getElementCountAndType(ArrType);
ArrayType *FlattenedArrayType = ArrayType::get(BaseType, TotalElements);
Value *PtrOperand = GEP.getPointerOperand();
Value *FlattenedIndex;
const DataLayout &DL = GEP.getDataLayout();
unsigned BitWidth = DL.getIndexTypeSizeInBits(GEP.getType());
APInt ConstantOffset(BitWidth, 0);
SmallMapVector<Value *, APInt, 4> VariableOffsets;
bool Success = GEP.collectOffset(DL, BitWidth, VariableOffsets, ConstantOffset);
(void)Success;
assert(Success && "Failed to collect offsets for GEP");
// GEP.collectOffset returns the offset in bytes. So we need to divide its
// offsets by the size in bytes of the BaseType
unsigned BaseTypeSizeInBytes = BaseType->getPrimitiveSizeInBits() / 8;
Value *ZeroIndex = Builder.getInt({BitWidth, 0});
FlattenedIndex = Builder.getInt(ConstantOffset.udiv(BaseTypeSizeInBytes));
for (auto [VarIndex, Multiplier] : VariableOffsets){
Value *ConstIntMul = Builder.getInt(Multiplier.udiv(BaseTypeSizeInBytes));
Value *MulVarIndex = Builder.CreateMul(VarIndex, ConstIntMul);
FlattenedIndex = Builder.CreateAdd(FlattenedIndex, MulVarIndex);
}
Value *NewGEP = Builder.CreateGEP(
FlattenedArrayType, PtrOperand, {ZeroIndex, FlattenedIndex},
GEP.getName(), GEP.getNoWrapFlags());
GEP.replaceAllUsesWith(NewGEP);
GEP.eraseFromParent();
return true;
}
```
This will flatten all GEPs, but not collapse any GEP chains. Again, the collapsing of GEP chains could be handled by the dxil-legalize pass (or a new pass).
_______________________________________________
llvm-bugs mailing list
llvm-bugs@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs