Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp | ||
---|---|---|
3833–3844 | I believe isUInt<32>() allows zero? | |
3848 | Changed.
Yes, intentionally. In the previous version, as long as the operand originates from a 32-bit scalar, we don't really care if zero-expansion matched. Though I couldn't provide a reproducer for the case without expansion. |
Should add some selector MIR tests
llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp | ||
---|---|---|
3833–3844 | Clearer to change to != 0 then |
Added an MIR instruction selection test and changed the if () condition
as suggested.
LGTM with a tweak to inst-select-load-smrd.mir.
llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp | ||
---|---|---|
3225 | Not your fault, but this seems to be missing checks (or asserts) on the number of operands and the type of the operands of the G_MERGE_VALUES. | |
3825–3826 | I don't understand this comment. What cases are we still missing? | |
llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-load-smrd.mir | ||
232 | Please put these checks before the function, not after. (But wow the existing checks in this file are really messy.) |
Not your fault, but this seems to be missing checks (or asserts) on the number of operands and the type of the operands of the G_MERGE_VALUES.