[Mlir-commits] [mlir] 78e17e7 - [mlir][arith][NFC] Make AtomicRMWKind switches exhaustive (#214622)

[llvmlistbot at llvm.org]

Author: 曾鈜寬 Tseng Hung Kuan
Date: 2026-08-09T11:03:48-04:00
New Revision: 78e17e70bd52058add1b8bfaeafa696b1e4e4cf5

URL: https://github.com/llvm/llvm-project/commit/78e17e70bd52058add1b8bfaeafa696b1e4e4cf5
DIFF: https://github.com/llvm/llvm-project/commit/78e17e70bd52058add1b8bfaeafa696b1e4e4cf5.diff

LOG: [mlir][arith][NFC] Make AtomicRMWKind switches exhaustive (#214622)

`getIdentityValueAttr` and `getReductionOp` in `ArithOps.cpp` each
handle 15 of
the 16 `AtomicRMWKind` cases and route the rest through a `default:`
label
carrying `// TODO: Add remaining reduction operations.`

That TODO cannot be completed. The only unhandled kind is `assign`,
which is
not a reduction: it has no identity element (`assign(x, e) = e`, so no
constant
`e` satisfies `assign(x, e) = x`) and no corresponding binary `arith`
op. It is
still a perfectly valid kind elsewhere — `memref.atomic_rmw` lowers it
to an
atomic `xchg` in `MemRefToLLVM.cpp` — it simply has no meaning for these
two
reduction helpers.

This patch spells `assign` out and drops the `default:`, which makes
both
switches exhaustive so that `-Wswitch` flags any kind added to the enum
later,
rather than leaving it to surface as a runtime diagnostic. The same
shape is
already used in `OpenACCUtilsReduction.cpp`.

**Why this is NFC.** The diagnostic moves below the switch rather than
into the
`assign` case, so every input behaves exactly as before. Keeping it
inside the
case would have dropped it for enum values outside the declared range,
which the
`default:` label used to catch.

**How the missing case was determined.** Not by reading: removing
`default:` and
compiling makes `-Wswitch` enumerate what is unhandled, and it reports
exactly
one value, `assign`, in each of the two switches.

**Why this went unnoticed.** The code slipped between both relevant
warnings:
`-Wswitch` is suppressed by the `default:` label, and
`-Wcovered-switch-default`
(which LLVM enables) does not apply because the switch is not in fact
fully
covered. This also brings the two switches in line with the coding
standard's
"Don't use default labels in fully covered switches over enumerations".

Tested with `ninja check-mlir` and targeted runs over the Arith,
Transforms,
Affine and OpenACC test directories.

This is my first contribution to LLVM — happy to adjust anything that
doesn't
match the project's conventions.

Added: 
    

Modified: 
    mlir/lib/Dialect/Arith/IR/ArithOps.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
index ff6a5d4a0c29a..fc50284fc8a94 100644
--- a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
+++ b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
@@ -3131,11 +3131,11 @@ TypedAttr mlir::arith::getIdentityValueAttr(AtomicRMWKind kind, Type resultType,
     return builder.getIntegerAttr(resultType, 1);
   case AtomicRMWKind::mulf:
     return builder.getFloatAttr(resultType, 1);
-  // TODO: Add remaining reduction operations.
-  default:
-    (void)emitOptionalError(loc, "Reduction operation type not supported");
+  // `assign` is not a reduction and has no identity element.
+  case AtomicRMWKind::assign:
     break;
   }
+  (void)emitOptionalError(loc, "Reduction operation type not supported");
   return nullptr;
 }
 
@@ -3226,11 +3226,11 @@ Value mlir::arith::getReductionOp(AtomicRMWKind op, OpBuilder &builder,
     return arith::AndIOp::create(builder, loc, lhs, rhs);
   case AtomicRMWKind::xori:
     return arith::XOrIOp::create(builder, loc, lhs, rhs);
-  // TODO: Add remaining reduction operations.
-  default:
-    (void)emitOptionalError(loc, "Reduction operation type not supported");
+  // `assign` is not a reduction and has no corresponding binary operation.
+  case AtomicRMWKind::assign:
     break;
   }
+  (void)emitOptionalError(loc, "Reduction operation type not supported");
   return nullptr;
 }