Skip to content

[MLIR][ROCDL] Add dynamically legal ops to LowerGpuOpsToROCDLOpsPass #108302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 12, 2024

Conversation

nirvedhmeshram
Copy link
Contributor

@nirvedhmeshram nirvedhmeshram commented Sep 11, 2024

Similar to #108266
After #102971
It is legal to generate LLVM::ExpOp and LLVM::LogOp if the type is is a float narrower than f64

@llvmbot
Copy link
Member

llvmbot commented Sep 11, 2024

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-gpu

Author: Nirvedh Meshram (nirvedhmeshram)

Changes

Similar to #108266
After #102971
It is legal to generate LLVM::ExpOp and LLVM::LogOp if the type is f32


Full diff: https://github.com/llvm/llvm-project/pull/108302.diff

1 Files Affected:

  • (modified) mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp (+8-1)
diff --git a/mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp b/mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp
index 29926719129dc5..1265f3f48a418e 100644
--- a/mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp
+++ b/mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp
@@ -332,7 +332,14 @@ void mlir::configureGpuToROCDLConversionLegality(ConversionTarget &target) {
   target.addIllegalOp<LLVM::CosOp, LLVM::ExpOp, LLVM::Exp2Op, LLVM::FCeilOp,
                       LLVM::FFloorOp, LLVM::FRemOp, LLVM::LogOp, LLVM::Log10Op,
                       LLVM::Log2Op, LLVM::PowOp, LLVM::SinOp>();
-
+  // These ops are legal for f32 type.
+  target.addDynamicallyLegalOp<LLVM::ExpOp, LLVM::LogOp>(
+      [](mlir::Operation *op) {
+        return llvm::any_of(op->getOperandTypes(), [](Type type) {
+          return llvm::isa<FloatType>(type) &&
+                 type.getIntOrFloatBitWidth() == 32;
+        });
+      });
   // TODO: Remove once we support replacing non-root ops.
   target.addLegalOp<gpu::YieldOp, gpu::GPUModuleOp>();
 }

@joker-eph
Copy link
Collaborator

Can you please add a test?

@nirvedhmeshram
Copy link
Contributor Author

Can you please add a test?

done!

nirvedhmeshram added a commit to iree-org/iree that referenced this pull request Sep 12, 2024
Bumps llvm to commit:
iree-org/llvm-project@e268afb
in branch
https://github.com/iree-org/llvm-project/tree/shared/integrates_20240910

Following changes are made:
1. Fix formatv call to pass validation added by
llvm/llvm-project#105745
2. API changes in DICompositeTypeAttr::get introduced by
llvm/llvm-project#106571
  3. Fix API call from llvm/llvm-project#100361
  4. Fix chipset comparison in ROCMTarget.cpp

There are two cherry-picks from upstream main as they contain fixes we
need and one cheery-pick that is yet to land
1.
iree-org/llvm-project@0d5d355
2.
iree-org/llvm-project@650d852
3.
iree-org/llvm-project@e268afb
(the upstream PR for this one is
llvm/llvm-project#108302

And a revert due to an outstanding torch-mlir issue

iree-org/llvm-project@cf22797
@llvmbot llvmbot added the bazel "Peripheral" support tier build system: utils/bazel label Sep 12, 2024
Copy link
Member

@kuhar kuhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nirvedhmeshram nirvedhmeshram merged commit a16164d into llvm:main Sep 12, 2024
6 of 7 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Sep 12, 2024

LLVM Buildbot has detected a new failure on builder mlir-rocm-mi200 running on mi200-buildbot while building mlir,utils at step 2 "checkout".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/177/builds/4875

Here is the relevant piece of the build log for the reference
Step 2 (checkout) failure: update (failure)
git version 2.25.1
fatal: unable to access 'https://github.com/llvm/llvm-project.git/': Could not resolve host: github.com
fatal: unable to access 'https://github.com/llvm/llvm-project.git/': Could not resolve host: github.com

@pcf000
Copy link
Contributor

pcf000 commented Sep 12, 2024

Transient failure, builds 4876 and 4877 had no problems.

josemonsalve2 pushed a commit to josemonsalve2/iree that referenced this pull request Sep 14, 2024
Bumps llvm to commit:
iree-org/llvm-project@e268afb
in branch
https://github.com/iree-org/llvm-project/tree/shared/integrates_20240910

Following changes are made:
1. Fix formatv call to pass validation added by
llvm/llvm-project#105745
2. API changes in DICompositeTypeAttr::get introduced by
llvm/llvm-project#106571
  3. Fix API call from llvm/llvm-project#100361
  4. Fix chipset comparison in ROCMTarget.cpp

There are two cherry-picks from upstream main as they contain fixes we
need and one cheery-pick that is yet to land
1.
iree-org/llvm-project@0d5d355
2.
iree-org/llvm-project@650d852
3.
iree-org/llvm-project@e268afb
(the upstream PR for this one is
llvm/llvm-project#108302

And a revert due to an outstanding torch-mlir issue

iree-org/llvm-project@cf22797
@nirvedhmeshram nirvedhmeshram deleted the rocdl_legal_2 branch September 20, 2024 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bazel "Peripheral" support tier build system: utils/bazel mlir:gpu mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants