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

[ftynse]

Looks good to me in general, please see inline.

mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir

48 ↗

(On Diff #236981)

What's the point of exercising the same operation twice?

[pifon2a]

mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir

48 ↗

(On Diff #236981)

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.

[merge_guards_bot]

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

[dfki-jugr]

Update clang-format issues.

[dfki-jugr]

Removed unnecessary test cases in test files.

[pifon2a]

Please, reformat the code and submit.

[rriddle]

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

Can we merge all of these instead?

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

mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp

66

Same here.

[dfki-jugr]

Merged template arguments in AddIllegalOP.

[ftynse]

Do you guys have commit access or should I land this for you?

[pifon2a]

In D72439#1816470, @ftynse wrote:

Do you guys have commit access or should I land this for you?

I have commit access, I can land it.

mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp

if (!m.getAttrOfType<UnitAttr>(gpu::GPUDialect::getKernelModuleAttrName()))
       return;
 
     OwningRewritePatternList patterns;
     NVVMTypeConverter converter(m.getContext());
     populateStdToLLVMConversionPatterns(converter, patterns);
     populateGpuToNVVMConversionPatterns(converter, patterns);
     ConversionTarget target(getContext());
     target.addIllegalDialect<gpu::GPUDialect>();
-    target.addIllegalOp<LLVM::FAbsOp>();
-    target.addIllegalOp<LLVM::FCeilOp>();
-    target.addIllegalOp<LLVM::CosOp>();
-    target.addIllegalOp<LLVM::ExpOp>();
+    target.addIllegalOp<LLVM::FAbsOp, LLVM::FCeilOp, LLVM::CosOp,
+                        LLVM::ExpOP>();
     target.addIllegalOp<FuncOp>();
     target.addLegalDialect<LLVM::LLVMDialect>();
     target.addLegalDialect<NVVM::NVVMDialect>();
     // TODO(csigg): Remove once we support replacing non-root ops.
     target.addLegalOp<gpu::YieldOp>();
     if (failed(applyPartialConversion(m, target, patterns, &converter)))
       signalPassFailure();
   }

mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp

patterns.insert<OpToFuncCallLowering<CeilFOp>>(converter, "_ocml_ceil_f32",
                                                  "_ocml_ceil_f64");
     patterns.insert<OpToFuncCallLowering<CosOp>>(converter, "_ocml_cos_f32",
                                                  "_ocml_cos_f64");
     patterns.insert<OpToFuncCallLowering<ExpOp>>(converter, "_ocml_exp_f32",
                                                  "_ocml_exp_f64");
 
     ConversionTarget target(getContext());
     target.addLegalDialect<LLVM::LLVMDialect, ROCDL::ROCDLDialect>();
-    target.addIllegalOp<LLVM::FAbsOp>();
-    target.addIllegalOp<LLVM::FCeilOp>();
-    target.addIllegalOp<LLVM::CosOp>();
-    target.addIllegalOp<LLVM::ExpOp>();
+    target.addIllegalOp<LLVM::FAbsOp, LLVM::FCeilOp, LLVM::CosOP,
+                        LLVM::ExpOp>();
     target.addDynamicallyLegalOp<FuncOp>(
         [&](FuncOp op) { return converter.isSignatureLegal(op.getType()); });
     if (failed(applyPartialConversion(m, target, patterns, &converter)))
       signalPassFailure();
   }
 };
 
 } // anonymous namespace
 
 std::unique_ptr<OpPassBase<ModuleOp>> mlir::createLowerGpuOpsToROCDLOpsPass() {
   return std::make_unique<LowerGpuOpsToROCDLOpsPass>();
 }
 
 static PassRegistration<LowerGpuOpsToROCDLOpsPass>
     pass("convert-gpu-to-rocdl",
          "Generate ROCDL operations for gpu operations");