Details
- Reviewers
herhut pifon2a ftynse - Commits
- rG202ab273e6ec: [mlir] Added missing GPU lowering ops.
Diff Detail
Event Timeline
Looks good to me in general, please see inline.
mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir | ||
---|---|---|
48 | What's the point of exercising the same operation twice? |
mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir | ||
---|---|---|
48 | I think this is copied from expf test where I was checking that the symbols are not added twice to the symbol table. I think here it is enough to exercise the op only once. |
Unit tests: pass. 61326 tests passed, 0 failed and 737 were skipped.
clang-tidy: fail. Please fix clang-tidy findings.
clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.
Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml
When uploading a diff, can you make sure to diff against HEAD instead of the previous commit? I couldn't see any of your changes and had to manually look through the diff history.
mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp | ||
---|---|---|
715 ↗ | (On Diff #236981) | Can we merge all of these instead? addIllegalOp<..., ..., ..., ...>(); |
mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp | ||
66 ↗ | (On Diff #236981) | Same here. |
What's the point of exercising the same operation twice?