Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
- Build Status
Buildable 35777 Build 35776: arc lint + arc unit
Event Timeline
Can you also remove the case for it in AMDGPURegisterBankInfo? It should never make it to RegBankSelected
| llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp | ||
|---|---|---|
| 1290–1295 | We don't really need to use F_CONSTANT. You can just use G_CONSTANT and the constant values directly without going through APFloat | |
| 1300 | Should propagate the fast math flags | |
| 1307–1310 | buildFMul | |
| llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp | ||
|---|---|---|
| 1292 | 1.0f | |
| llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp | ||
|---|---|---|
| 659 | Should default to None. I'm not sure if this one should get a flags argument. You can set it on the instruction separately from constructing it | |
| 671 | Should default to None | |
| 701 | Should default to None | |
| 710 | Should default to None | |
| llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-intrinsic-amdgcn-fdiv-fast.mir | ||
| 29 | Should add another test with the flags | |
| llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp | ||
|---|---|---|
| 659 | The defaults are in the header. Isn't it more consistent if you can set the flags when constructing it? | |
LGTM with fabs flags fixed
| llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp | ||
|---|---|---|
| 1289 | Missing flags | |
Should default to None.
I'm not sure if this one should get a flags argument. You can set it on the instruction separately from constructing it