Skip to content

Conversation

@matthias-springer
Copy link
Member

This pattern used to create an op and then attached the converted rounding mode attribute. When the latter failed, the pattern aborted and a rollback was triggered.

This commit inverses the logic: the converted rounding mode is computed first, so that no changes have to be rolled back.

Note: This is in preparation of the One-Shot Dialect Conversion refactoring.

This pattern used to create an op and then attached the converted rounding mode attribute. When the latter failed, a rollback was triggered.

This commit inverses the logic: the converted rounding mode is computed first, so that no changes have to be rolled back.

Note: This is in preparation of the One-Shot Dialect Conversion refactoring.
@llvmbot
Copy link
Member

llvmbot commented Apr 18, 2025

@llvm/pr-subscribers-mlir-spirv

@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)

Changes

This pattern used to create an op and then attached the converted rounding mode attribute. When the latter failed, the pattern aborted and a rollback was triggered.

This commit inverses the logic: the converted rounding mode is computed first, so that no changes have to be rolled back.

Note: This is in preparation of the One-Shot Dialect Conversion refactoring.


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

1 Files Affected:

  • (modified) mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp (+12-8)
diff --git a/mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp b/mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp
index 9c4dfa27b1447..434d7df853a5e 100644
--- a/mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp
+++ b/mlir/lib/Conversion/ArithToSPIRV/ArithToSPIRV.cpp
@@ -847,24 +847,28 @@ struct TypeCastingOpPattern final : public OpConversionPattern<Op> {
       // Then we can just erase this operation by forwarding its operand.
       rewriter.replaceOp(op, adaptor.getOperands().front());
     } else {
-      auto newOp = rewriter.template replaceOpWithNewOp<SPIRVOp>(
-          op, dstType, adaptor.getOperands());
+      // Compute new rounding mode (if any).
+      std::optional<spirv::FPRoundingMode> rm = std::nullopt;
       if (auto roundingModeOp =
               dyn_cast<arith::ArithRoundingModeInterface>(*op)) {
         if (arith::RoundingModeAttr roundingMode =
                 roundingModeOp.getRoundingModeAttr()) {
-          if (auto rm =
-                  convertArithRoundingModeToSPIRV(roundingMode.getValue())) {
-            newOp->setAttr(
-                getDecorationString(spirv::Decoration::FPRoundingMode),
-                spirv::FPRoundingModeAttr::get(rewriter.getContext(), *rm));
-          } else {
+          if (!(rm =
+                    convertArithRoundingModeToSPIRV(roundingMode.getValue()))) {
             return rewriter.notifyMatchFailure(
                 op->getLoc(),
                 llvm::formatv("unsupported rounding mode '{0}'", roundingMode));
           }
         }
       }
+      // Create replacement op and attach rounding mode attribute (if any).
+      auto newOp = rewriter.template replaceOpWithNewOp<SPIRVOp>(
+          op, dstType, adaptor.getOperands());
+      if (rm) {
+        newOp->setAttr(
+            getDecorationString(spirv::Decoration::FPRoundingMode),
+            spirv::FPRoundingModeAttr::get(rewriter.getContext(), *rm));
+      }
     }
     return success();
   }

@matthias-springer matthias-springer merged commit c47042c into main Apr 19, 2025
14 checks passed
@matthias-springer matthias-springer deleted the users/matthias-springer/no_rollback_spirv_cast branch April 19, 2025 08:01
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
)

This pattern used to create an op and then attached the converted
rounding mode attribute. When the latter failed, the pattern aborted and
a rollback was triggered.

This commit inverses the logic: the converted rounding mode is computed
first, so that no changes have to be rolled back.

Note: This is in preparation of the One-Shot Dialect Conversion
refactoring.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants