[PATCH] D72439: [mlir] Added Abs, Ceil and Cos to GPU lowering conversion.

Thu Jan 9 10:33:25 PST 2020

rriddle added a comment.

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.

================
Comment at: mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp:715
     target.addIllegalDialect<gpu::GPUDialect>();
+    target.addIllegalOp<LLVM::FAbsOp>();
+    target.addIllegalOp<LLVM::FCeilOp>();
----------------
Can we merge all of these instead?

addIllegalOp<..., ..., ..., ...>();

================
Comment at: mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp:66
+    target.addIllegalOp<LLVM::FAbsOp>();
+    target.addIllegalOp<LLVM::FCeilOp>();
+    target.addIllegalOp<LLVM::CosOp>();
----------------
Same here.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72439/new/

https://reviews.llvm.org/D72439