Skip to content

Conversation

keshavvinayak01
Copy link
Contributor

@keshavvinayak01 keshavvinayak01 commented Sep 22, 2025

Description

Added Pattern for lowering Math::ClampFOp to ROCDL::FMED3.
Also added chipet option to MathToRocdl pass to check for arch support ISA instructions

Solves #15072

Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@keshavvinayak01 keshavvinayak01 changed the title [WIP][ROCDL] ]Added math.clampf -> rocdl.fmed3 conversion{ [MLIR][ROCDL] ]Added math.clampf -> rocdl.fmed3 conversion{ Sep 23, 2025
@keshavvinayak01 keshavvinayak01 changed the title [MLIR][ROCDL] ]Added math.clampf -> rocdl.fmed3 conversion{ [MLIR][ROCDL] Added math.clampf -> rocdl.fmed3 conversion{ Sep 23, 2025
@keshavvinayak01 keshavvinayak01 marked this pull request as ready for review September 23, 2025 16:31
@keshavvinayak01
Copy link
Contributor Author

@Groverkss

@llvmbot llvmbot added the mlir label Sep 23, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 23, 2025

@llvm/pr-subscribers-mlir

Author: Keshav Vinayak Jha (keshavvinayak01)

Changes

Description

Added Pattern for lowering Math::ClampFOp to ROCDL::FMED3.
Also added chipet option to MathToRocdl pass to check for arch support ISA instructions

Solves #15072


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

3 Files Affected:

  • (modified) mlir/include/mlir/Conversion/Passes.td (+6)
  • (modified) mlir/lib/Conversion/MathToROCDL/MathToROCDL.cpp (+34-7)
  • (modified) mlir/test/Conversion/MathToROCDL/math-to-rocdl.mlir (+32-1)
diff --git a/mlir/include/mlir/Conversion/Passes.td b/mlir/include/mlir/Conversion/Passes.td
index 1a37d057776e2..060c7183fcb3a 100644
--- a/mlir/include/mlir/Conversion/Passes.td
+++ b/mlir/include/mlir/Conversion/Passes.td
@@ -785,6 +785,12 @@ def ConvertMathToROCDL : Pass<"convert-math-to-rocdl", "ModuleOp"> {
     "ROCDL::ROCDLDialect",
     "vector::VectorDialect",
   ];
+  let options = [
+    Option<"chipset", "chipset", "std::string",
+          /*default=*/"\"gfx000\"",
+          "Chipset that these operations will run on">
+  ];
+
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Conversion/MathToROCDL/MathToROCDL.cpp b/mlir/lib/Conversion/MathToROCDL/MathToROCDL.cpp
index df219f3ff4f6e..6da7e9a850ef7 100644
--- a/mlir/lib/Conversion/MathToROCDL/MathToROCDL.cpp
+++ b/mlir/lib/Conversion/MathToROCDL/MathToROCDL.cpp
@@ -10,6 +10,7 @@
 #include "mlir/Conversion/GPUCommon/GPUCommonPass.h"
 #include "mlir/Conversion/LLVMCommon/LoweringOptions.h"
 #include "mlir/Conversion/LLVMCommon/TypeConverter.h"
+#include "mlir/Dialect/AMDGPU/Utils/Chipset.h"
 #include "mlir/Dialect/Func/IR/FuncOps.h"
 #include "mlir/Dialect/LLVMIR/LLVMDialect.h"
 #include "mlir/Dialect/LLVMIR/ROCDLDialect.h"
@@ -120,25 +121,51 @@ void mlir::populateMathToROCDLConversionPatterns(
                                     "__ocml_fmod_f64", "__ocml_fmod_f16");
 }
 
-namespace {
-struct ConvertMathToROCDLPass
-    : public impl::ConvertMathToROCDLBase<ConvertMathToROCDLPass> {
-  ConvertMathToROCDLPass() = default;
+struct ClampFOpConversion : public ConvertOpToLLVMPattern<math::ClampFOp> {
+  using ConvertOpToLLVMPattern::ConvertOpToLLVMPattern;
+  ClampFOpConversion(const LLVMTypeConverter &converter,
+                     amdgpu::Chipset chipset)
+      : ConvertOpToLLVMPattern<math::ClampFOp>(converter), chipset(chipset) {}
+
+  LogicalResult
+  matchAndRewrite(math::ClampFOp op, OpAdaptor adaptor,
+                  ConversionPatternRewriter &rewriter) const override {
+    // V_MED3_F16/F32 only exists in gfx9+ artchitectures
+    if (chipset.majorVersion < 9) {
+      std::string msg =
+          ("pre-gfx9 (gfx" + std::to_string(chipset.majorVersion) +
+           "): V_MED_F16 / V_MED3_F32 not supported.");
+      return rewriter.notifyMatchFailure(op, msg);
+    }
+    rewriter.replaceOpWithNewOp<ROCDL::FMed3Op>(op, op.getType(), op.getValue(),
+                                                op.getMin(), op.getMax());
+    return success();
+  }
+  amdgpu::Chipset chipset;
+};
+
+struct ConvertMathToROCDLPass final
+    : impl::ConvertMathToROCDLBase<ConvertMathToROCDLPass> {
+  using impl::ConvertMathToROCDLBase<
+      ConvertMathToROCDLPass>::ConvertMathToROCDLBase;
+
   void runOnOperation() override;
 };
-} // namespace
 
 void ConvertMathToROCDLPass::runOnOperation() {
   auto m = getOperation();
   MLIRContext *ctx = m.getContext();
+  FailureOr<amdgpu::Chipset> maybeChipset = amdgpu::Chipset::parse(chipset);
 
   RewritePatternSet patterns(&getContext());
   LowerToLLVMOptions options(ctx, DataLayout(m));
   LLVMTypeConverter converter(ctx, options);
+  patterns.add<ClampFOpConversion>(converter, *maybeChipset);
   populateMathToROCDLConversionPatterns(converter, patterns);
   ConversionTarget target(getContext());
-  target.addLegalDialect<BuiltinDialect, func::FuncDialect,
-                         vector::VectorDialect, LLVM::LLVMDialect>();
+  target
+      .addLegalDialect<BuiltinDialect, func::FuncDialect, vector::VectorDialect,
+                       LLVM::LLVMDialect, ROCDL::ROCDLDialect>();
   target.addIllegalOp<LLVM::CosOp, LLVM::ExpOp, LLVM::Exp2Op, LLVM::FAbsOp,
                       LLVM::FCeilOp, LLVM::FFloorOp, LLVM::FRemOp, LLVM::LogOp,
                       LLVM::Log10Op, LLVM::Log2Op, LLVM::PowOp, LLVM::SinOp,
diff --git a/mlir/test/Conversion/MathToROCDL/math-to-rocdl.mlir b/mlir/test/Conversion/MathToROCDL/math-to-rocdl.mlir
index dbff23339d8b3..541d8d53cac4c 100644
--- a/mlir/test/Conversion/MathToROCDL/math-to-rocdl.mlir
+++ b/mlir/test/Conversion/MathToROCDL/math-to-rocdl.mlir
@@ -1,4 +1,5 @@
-// RUN: mlir-opt %s -convert-math-to-rocdl -allow-unregistered-dialect -split-input-file | FileCheck %s
+// RUN: mlir-opt %s -allow-unregistered-dialect -split-input-file -pass-pipeline='builtin.module(convert-math-to-rocdl{chipset=gfx803})' | FileCheck %s --check-prefix=PRE9
+// RUN: mlir-opt %s -allow-unregistered-dialect -split-input-file -pass-pipeline='builtin.module(convert-math-to-rocdl{chipset=gfx942})' | FileCheck %s --check-prefix=POST9
 
 module @test_module {
   // CHECK: llvm.func @__ocml_fmod_f16(f16, f16) -> f16
@@ -596,3 +597,33 @@ module @test_module {
     func.return %result : vector<2x2xf16>
   }
 }
+
+// -----
+
+// f16 clamp → rocdl.fmed3 on gfx9+
+func.func @clampf_f16(%x: f16, %lo: f16, %hi: f16) -> f16 {
+  %r = math.clampf %x to [%lo, %hi] : f16
+  return %r : f16
+}
+
+// f32 clamp → rocdl.fmed3 on gfx9+
+func.func @clampf_f32(%x: f32, %lo: f32, %hi: f32) -> f32 {
+  %r = math.clampf %x to [%lo, %hi] : f32
+  return %r : f32
+}
+
+// POST9-LABEL: func.func @clampf_f16
+// POST9: rocdl.fmed3 {{.*}} : f16
+// POST9: return
+
+// POST9-LABEL: func.func @clampf_f32
+// POST9: rocdl.fmed3 {{.*}} : f32
+// POST9: return
+
+// PRE9-LABEL: func.func @clampf_f16
+// PRE9-NOT: rocdl.fmed3
+// PRE9: math.clampf {{.*}} : f16
+
+// PRE9-LABEL: func.func @clampf_f32
+// PRE9-NOT: rocdl.fmed3
+// PRE9: math.clampf {{.*}} : f32

@Groverkss Groverkss requested a review from krzysz00 September 26, 2025 14:59
Copy link
Member

@Groverkss Groverkss left a comment

Choose a reason for hiding this comment

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

LGTM, but probably should get a review from @krzysz00

joker-eph and others added 20 commits October 1, 2025 04:50
We were converting the `ASInt` to as sign-less `APInt` too early and
losing the sign information.
The `distinct_objects` operation takes a list of memrefs and returns a
list of memrefs of the same types, with the additional assumption that
accesses to these memrefs will never alias with each other. This means
that loads and stores to different memrefs in the list can be safely
reordered.

The discussion
https://discourse.llvm.org/t/rfc-introducing-memref-aliasing-attributes/88049
…undtrip (llvm#161499)

We've been seen (very sporadic) lifetime issues around this area. Here's
an example backtrace:
```
[  8] 0x0000000188e56743 libsystem_platform.dylib`_sigtramp + 55
[  9] 0x00000001181e041f LLDB`lldb_private::CPlusPlusLanguage::SymbolNameFitsToLanguage(lldb_private::Mangled) const [inlined] unsigned long std::1::constexpr_strlen[abi:nn200100]<char>(char const*) + 7 at constexpr_c_functions.h:63:10
[  9] 0x00000001181e0418 LLDB`lldb_private::CPlusPlusLanguage::SymbolNameFitsToLanguage(lldb_private::Mangled) const [inlined] std::__1::char_traits<char>::length[abi:nn200100](char const*) at char_traits.h:232:12
[  9] 0x00000001181e0418 LLDB`lldb_private::CPlusPlusLanguage::SymbolNameFitsToLanguage(lldb_private::Mangled) const [inlined] llvm::StringRef::StringRef(char const*) at StringRef.h:90:33
[  9] 0x00000001181e0418 LLDB`lldb_private::CPlusPlusLanguage::SymbolNameFitsToLanguage(lldb_private::Mangled) const [inlined] llvm::StringRef::StringRef(char const*) at StringRef.h:92:38
[  9] 0x00000001181e0418 LLDB`lldb_private::CPlusPlusLanguage::SymbolNameFitsToLanguage(lldb_private::Mangled) const + 20 at CPlusPlusLanguage.cpp:68:62
```

Looks like we're calling `strlen` on a nullptr. I stared at this
codepath for a while but am still not sure how that could happen unless
the underlying `ConstString` somehow pointed to corrupted data.

But `SymbolNameFitsToLanguage` does some roundtripping through a `const
char*` before calling `GetManglingScheme`. No other callsite does this
and it just seems redundant.

This patch cleans this up.

rdar://161128180
…llvm#161456)

Add option to `WriteAsOperandInternal` to print the type and use that to
eliminate explicit type printing code in several places.
…llvm#161498)

This is unused. Targets can lower/expand the `PARTIAL_REDUCE_*` ISD
nodes.
…161382)

The previous patch ensured that we correctly got the allocas put in
place. This patch takes the address of each element of each alloca, and
copies it to the previous one. This allows us to re-form the
pointer-structure for a recipe.
…lvm#160724)

This op enables expressing uncertainty regarding what should be
happening at particular places in transform-dialect schedules. In
particular, it enables representing a choice among alternative regions.
This choice is resolved through providing a `selected_region` argument.
When this argument is provided, the semantics are such that it is valid
to rewrite the op through substituting in the selected region -- with
the op's interpreted semantics corresponding to exactly this.

This op represents another piece of the puzzle w.r.t. a toolkit for
expressing autotuning problems with the transform dialect. Note that
this goes beyond tuning knobs _on_ transforms, going further by making
it tunable which (sequences of) transforms are to be applied.
Add tests which show missed folds of subregister extracts with
intermediate full copies.
…insics (llvm#161518)

PACKSS intrinsic calls are only expanded to X86ISD::PACKSS nodes during
legalisation, after which time we fail to handle cases where ASHR sign
splats (now lowered to X86ISD::VSRAI) are unnecessary.

Add additional example of FREEZE(PACKSS()) as that's an issue as well.
)

Self-replacing has a different meaning in InstCombine. It will replace
all uses with poison.
Closes llvm#161492.
…lvm#160987)

Fixes llvm#160250
We previously assumed the select to unfold is defined in the incoming
block of phi user, as `isValidSelectInst` filters other cases at the
initial stage. However, the selects not defined in the incoming block
may occur after unfolding the arms of the unfolded select.
This patch sinks the select into the incoming block of the phi user and
unfolds it at the incoming block.
We see performance improvements from using sincos to reuse calculations
in hot loops that compute sin() and cos() of the same operand. Add a
pass to identify sin() and cos() calls in the same block with the same
operand and fast-math flags, and fuse them into a sincos op.

Follow-up to:
* llvm#160561
* llvm#160772
)

Replace the target specific copy with a call to the generic routine. I
don't spot any differences by eye, and there's nothing in the original
review discussion (llvm#124327) which makes it clear why this was
duplicated.
…61505)

These tests show that `luti4` intrinsics are currently incorrectly
CSD'd.
llvm#161314)

Since the second argument must be a constant integer, we can as well
convert it to a `ConstantExpr` in Sema.


Fixes llvm#161272
…ndling when only demanding 'known signbits' (llvm#161523)

If we only demand bits that already match the signbit then we don't need to shift.

Generalizes an existing pattern that just handled signbit-only demanded bits to match what we do for ISD::SRA.
keshavvinayak01 and others added 4 commits October 13, 2025 11:19
1. Added the 'final' keyword to ClampFOpConversion
2. Removed string variable; directly add the message string to notifyMatchFailure.
3. Added chipset argument to populateMathToROCDLConversionPatterns instead.

Signed-off-by: Keshav Vinayak Jha <[email protected]>
@keshavvinayak01
Copy link
Contributor Author

Incorrect rebase; will move the PR

krzysz00 pushed a commit that referenced this pull request Oct 14, 2025
Added Pattern for lowering `Math::ClampFOp` to `ROCDL::FMED3`.
Also added `chipset` option to `MathToRocdl` pass to check for arch
support ISA instructions

Solves [#15072](#157052)

Reapplies #160100

---------

Signed-off-by: Keshav Vinayak Jha <[email protected]>
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Oct 14, 2025
…163259)

Added Pattern for lowering `Math::ClampFOp` to `ROCDL::FMED3`.
Also added `chipset` option to `MathToRocdl` pass to check for arch
support ISA instructions

Solves [#15072](llvm/llvm-project#157052)

Reapplies llvm/llvm-project#160100

---------

Signed-off-by: Keshav Vinayak Jha <[email protected]>
akadutta pushed a commit to akadutta/llvm-project that referenced this pull request Oct 14, 2025
Added Pattern for lowering `Math::ClampFOp` to `ROCDL::FMED3`.
Also added `chipset` option to `MathToRocdl` pass to check for arch
support ISA instructions

Solves [llvm#15072](llvm#157052)

Reapplies llvm#160100

---------

Signed-off-by: Keshav Vinayak Jha <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.