| Issue |
185149
|
| Summary |
[MLIR][SPIRV] SPIRVConversion.h: bug in getPushConstantValue
|
| Labels |
mlir
|
| Assignees |
|
| Reporter |
Emimendoza
|
I think I found a bug in the spirv dialect but I am unsure of how to implement a solution. So SPIRVConversion.h has this utility function:
https://github.com/llvm/llvm-project/blob/69902769c74718295dd553a1cd0bb6c56ff2645a/mlir/include/mlir/Dialect/SPIRV/Transforms/SPIRVConversion.h#L162-L169
and the description explicitly states that the push constant has `integerType` integers.
The implementation relies on the assumption that integerType is in fact 32bit. It contains the lines:
https://github.com/llvm/llvm-project/blob/69902769c74718295dd553a1cd0bb6c56ff2645a/mlir/lib/Dialect/SPIRV/Transforms/SPIRVConversion.cpp#L1323-L1324
Which ends up producing mlir that looks like this (when 64 bit integer is used):
```mlir
%3 = "spirv.Constant"() <{value = 0 : i32}> : () -> i64
```
which causes the pass created by `createSPIRVLowerABIAttributesPass` to fail.
The assumption of integerType being 32bits isn't just used by that one line.
getPushConstantValue implementation also uses functions such as:
https://github.com/llvm/llvm-project/blob/69902769c74718295dd553a1cd0bb6c56ff2645a/mlir/lib/Dialect/SPIRV/Transforms/SPIRVConversion.cpp#L965-L970
which ends up calling other functions that also assume 32bit integer types, just having a parameter specifying the index type.
Now I don't know if this is a documentation bug. As in `getPushConstantValue` was meant to use `integerType` as an index type like all the other functions, or if `getPushConstantValue` was meant to behave the way its description is written and all the static functions implementing its machinery just assume 32bit `integerType`?
![]()
_______________________________________________
llvm-bugs mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs
