[llvm-branch-commits] [flang] [flang][CodeGen] Replace fir.select* FIR-to-LLVM patterns with stubs that emit conversion errors (PR #212978)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Jul 30 02:52:14 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-fir-hlfir
Author: Kareem Ergawy (ergawy)
<details>
<summary>Changes</summary>
`fir.select`, `fir.select_case`, `fir.select_rank`, and `fir.select_type` are lowered to cf.* earlier in the pipeline (`--fir-select-ops-conversion` and `--fir-polymorphic-op`). Their FIR-to-LLVM conversion patterns are dead in a correct pipeline. Replace them with a single templated stub `SelectShouldHaveBeenConvertedStub<OP>` that emits `"'fir.<op>' op should have already been converted"` and fails legalization, so running `--fir-to-llvm-ir` standalone on stale IR reports a clear diagnostic instead of "unable to legalize".
`Fir/convert-to-llvm.fir`'s six select* test blocks are removed (the lowering no longer runs; CF-level coverage lives in `Fir/SelectOpsConversion/`). `Fir/convert-to-llvm-invalid.fir` gains a stub-error test per op. `Fir/Todo/select_case_with_character.fir` is retargeted to check the equivalent diagnostic now emitted by `--fir-select-ops-conversion`.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@<!-- -->anthropic.com>
---
Patch is 29.34 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/212978.diff
4 Files Affected:
- (modified) flang/lib/Optimizer/CodeGen/CodeGen.cpp (+19-258)
- (modified) flang/test/Fir/Todo/select_case_with_character.fir (+1-1)
- (modified) flang/test/Fir/convert-to-llvm-invalid.fir (+41-4)
- (modified) flang/test/Fir/convert-to-llvm.fir (-311)
``````````diff
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index 483f839e5b666..4da07ace59863 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -3972,272 +3972,33 @@ struct ModuleDebugImportsOpConversion
}
};
-static void genCondBrOp(mlir::Location loc, mlir::Value cmp, mlir::Block *dest,
- std::optional<mlir::ValueRange> destOps,
- mlir::ConversionPatternRewriter &rewriter,
- mlir::Block *newBlock) {
- if (destOps)
- mlir::LLVM::CondBrOp::create(rewriter, loc, cmp, dest, *destOps, newBlock,
- mlir::ValueRange());
- else
- mlir::LLVM::CondBrOp::create(rewriter, loc, cmp, dest, newBlock);
-}
-
-template <typename A, typename B>
-static void genBrOp(A caseOp, mlir::Block *dest, std::optional<B> destOps,
- mlir::ConversionPatternRewriter &rewriter) {
- if (destOps)
- rewriter.replaceOpWithNewOp<mlir::LLVM::BrOp>(caseOp, *destOps, dest);
- else
- rewriter.replaceOpWithNewOp<mlir::LLVM::BrOp>(caseOp, B{}, dest);
-}
-
-static void genCaseLadderStep(mlir::Location loc, mlir::Value cmp,
- mlir::Block *dest,
- std::optional<mlir::ValueRange> destOps,
- mlir::ConversionPatternRewriter &rewriter) {
- auto *thisBlock = rewriter.getInsertionBlock();
- auto *newBlock = createBlock(rewriter, dest);
- rewriter.setInsertionPointToEnd(thisBlock);
- genCondBrOp(loc, cmp, dest, destOps, rewriter, newBlock);
- rewriter.setInsertionPointToEnd(newBlock);
-}
-
-/// Conversion of `fir.select_case`
-///
-/// The `fir.select_case` operation is converted to a if-then-else ladder.
-/// Depending on the case condition type, one or several comparison and
-/// conditional branching can be generated.
-///
-/// A point value case such as `case(4)`, a lower bound case such as
-/// `case(5:)` or an upper bound case such as `case(:3)` are converted to a
-/// simple comparison between the selector value and the constant value in the
-/// case. The block associated with the case condition is then executed if
-/// the comparison succeed otherwise it branch to the next block with the
-/// comparison for the next case conditon.
-///
-/// A closed interval case condition such as `case(7:10)` is converted with a
-/// first comparison and conditional branching for the lower bound. If
-/// successful, it branch to a second block with the comparison for the
-/// upper bound in the same case condition.
-///
-/// TODO: lowering of CHARACTER type cases is not handled yet.
-struct SelectCaseOpConversion : public fir::FIROpConversion<fir::SelectCaseOp> {
- using FIROpConversion::FIROpConversion;
-
- llvm::LogicalResult
- matchAndRewrite(fir::SelectCaseOp caseOp, OpAdaptor adaptor,
- mlir::ConversionPatternRewriter &rewriter) const override {
- unsigned conds = caseOp.getNumConditions();
- llvm::ArrayRef<mlir::Attribute> cases = caseOp.getCases().getValue();
- // Type can be CHARACTER, INTEGER, or LOGICAL (C1145)
- auto ty = caseOp.getSelector().getType();
- if (mlir::isa<fir::CharacterType>(ty)) {
- TODO(caseOp.getLoc(), "fir.select_case codegen with character type");
- return mlir::failure();
- }
- mlir::Value selector = caseOp.getSelector(adaptor.getOperands());
- auto loc = caseOp.getLoc();
- for (unsigned t = 0; t != conds; ++t) {
- mlir::Block *dest = caseOp.getSuccessor(t);
- std::optional<mlir::ValueRange> destOps =
- caseOp.getSuccessorOperands(adaptor.getOperands(), t);
- // Convert block signature if needed
- if (destOps && !destOps->empty())
- if (auto conversion = getTypeConverter()->convertBlockSignature(dest))
- dest = rewriter.applySignatureConversion(dest, *conversion,
- getTypeConverter());
- std::optional<mlir::ValueRange> cmpOps =
- *caseOp.getCompareOperands(adaptor.getOperands(), t);
- mlir::Attribute attr = cases[t];
- assert(mlir::isa<mlir::UnitAttr>(attr) || cmpOps.has_value());
- if (mlir::isa<fir::PointIntervalAttr>(attr)) {
- auto cmp = mlir::LLVM::ICmpOp::create(rewriter, loc,
- mlir::LLVM::ICmpPredicate::eq,
- selector, cmpOps->front());
- genCaseLadderStep(loc, cmp, dest, destOps, rewriter);
- continue;
- }
- if (mlir::isa<fir::LowerBoundAttr>(attr)) {
- auto cmp = mlir::LLVM::ICmpOp::create(rewriter, loc,
- mlir::LLVM::ICmpPredicate::sle,
- cmpOps->front(), selector);
- genCaseLadderStep(loc, cmp, dest, destOps, rewriter);
- continue;
- }
- if (mlir::isa<fir::UpperBoundAttr>(attr)) {
- auto cmp = mlir::LLVM::ICmpOp::create(rewriter, loc,
- mlir::LLVM::ICmpPredicate::sle,
- selector, cmpOps->front());
- genCaseLadderStep(loc, cmp, dest, destOps, rewriter);
- continue;
- }
- if (mlir::isa<fir::ClosedIntervalAttr>(attr)) {
- mlir::Value caseArg0 = *cmpOps->begin();
- auto cmp0 = mlir::LLVM::ICmpOp::create(
- rewriter, loc, mlir::LLVM::ICmpPredicate::sle, caseArg0, selector);
- auto *thisBlock = rewriter.getInsertionBlock();
- auto *newBlock1 = createBlock(rewriter, dest);
- auto *newBlock2 = createBlock(rewriter, dest);
- rewriter.setInsertionPointToEnd(thisBlock);
- mlir::LLVM::CondBrOp::create(rewriter, loc, cmp0, newBlock1, newBlock2);
- rewriter.setInsertionPointToEnd(newBlock1);
- mlir::Value caseArg1 = *(cmpOps->begin() + 1);
- auto cmp1 = mlir::LLVM::ICmpOp::create(
- rewriter, loc, mlir::LLVM::ICmpPredicate::sle, selector, caseArg1);
- genCondBrOp(loc, cmp1, dest, destOps, rewriter, newBlock2);
- rewriter.setInsertionPointToEnd(newBlock2);
- continue;
- }
- assert(mlir::isa<mlir::UnitAttr>(attr));
- assert((t + 1 == conds) && "unit must be last");
- genBrOp(caseOp, dest, destOps, rewriter);
- }
- return mlir::success();
- }
-};
-
-/// Base class for SelectOpConversion and SelectRankOpConversion.
+/// Defensive stub. `fir.select`, `fir.select_case`, and `fir.select_rank` are
+/// lowered to `cf.switch` / `cf.cond_br` by `--fir-select-ops-conversion`
+/// earlier in the pipeline; `fir.select_type` is lowered to a cf-based
+/// if-then-else ladder by `--fir-polymorphic-op`. If any of these ops
+/// reaches FIR-to-LLVM, we emit a clear diagnostic rather than an opaque
+/// "unable to legalize" error.
template <typename OP>
-struct SelectOpConversionBase : public fir::FIROpConversion<OP> {
+struct SelectShouldHaveBeenConvertedStub : public fir::FIROpConversion<OP> {
using fir::FIROpConversion<OP>::FIROpConversion;
-private:
- /// Helper function for converting select ops. This function converts the
- /// signature of the given block. If the new block signature is different from
- /// `expectedTypes`, returns "failure".
- llvm::FailureOr<mlir::Block *>
- getConvertedBlock(mlir::ConversionPatternRewriter &rewriter,
- mlir::Operation *branchOp, mlir::Block *block,
- mlir::TypeRange expectedTypes) const {
- const mlir::TypeConverter *converter = this->getTypeConverter();
- assert(converter && "expected non-null type converter");
- assert(!block->isEntryBlock() && "entry blocks have no predecessors");
-
- // There is nothing to do if the types already match.
- if (block->getArgumentTypes() == expectedTypes)
- return block;
-
- // Compute the new block argument types and convert the block.
- std::optional<mlir::TypeConverter::SignatureConversion> conversion =
- converter->convertBlockSignature(block);
- if (!conversion)
- return rewriter.notifyMatchFailure(branchOp,
- "could not compute block signature");
- if (expectedTypes != conversion->getConvertedTypes())
- return rewriter.notifyMatchFailure(branchOp,
- "mismatch between adaptor operand "
- "types and computed block signature");
- return rewriter.applySignatureConversion(block, *conversion, converter);
- }
-
-protected:
- llvm::LogicalResult
- selectMatchAndRewrite(OP select, typename OP::Adaptor adaptor,
- mlir::ConversionPatternRewriter &rewriter) const {
- unsigned conds = select.getNumConditions();
- auto cases = select.getCases().getValue();
- mlir::Value selector = adaptor.getSelector();
- auto loc = select.getLoc();
- assert(conds > 0 && "select must have cases");
-
- llvm::SmallVector<mlir::Block *> destinations;
- llvm::SmallVector<mlir::ValueRange> destinationsOperands;
- mlir::Block *defaultDestination;
- mlir::ValueRange defaultOperands;
- // LLVM::SwitchOp selector type and the case values types
- // must have the same bit width, so cast the selector to i64,
- // and use i64 for the case values. It is hard to imagine
- // a computed GO TO with the number of labels in the label-list
- // bigger than INT_MAX, but let's use i64 to be on the safe side.
- // Moreover, fir.select operation is more relaxed than
- // a Fortran computed GO TO, so it may specify such a case value
- // even if there is just a single label/case.
- llvm::SmallVector<int64_t> caseValues;
-
- for (unsigned t = 0; t != conds; ++t) {
- mlir::Block *dest = select.getSuccessor(t);
- auto destOps = select.getSuccessorOperands(adaptor.getOperands(), t);
- const mlir::Attribute &attr = cases[t];
- if (auto intAttr = mlir::dyn_cast<mlir::IntegerAttr>(attr)) {
- destinationsOperands.push_back(destOps ? *destOps : mlir::ValueRange{});
- auto convertedBlock =
- getConvertedBlock(rewriter, select, dest,
- mlir::TypeRange(destinationsOperands.back()));
- if (mlir::failed(convertedBlock))
- return mlir::failure();
- destinations.push_back(*convertedBlock);
- caseValues.push_back(intAttr.getInt());
- continue;
- }
- assert(mlir::dyn_cast_or_null<mlir::UnitAttr>(attr));
- assert((t + 1 == conds) && "unit must be last");
- defaultOperands = destOps ? *destOps : mlir::ValueRange{};
- auto convertedBlock = getConvertedBlock(rewriter, select, dest,
- mlir::TypeRange(defaultOperands));
- if (mlir::failed(convertedBlock))
- return mlir::failure();
- defaultDestination = *convertedBlock;
- }
-
- // Deal with the case where there is only a default destination. Handle it
- // now because emitting empty case values is not legal.
- if (caseValues.empty()) {
- rewriter.replaceOpWithNewOp<mlir::LLVM::BrOp>(select, defaultOperands,
- defaultDestination);
- return mlir::success();
- }
-
- selector =
- this->integerCast(loc, rewriter, rewriter.getI64Type(), selector);
-
- rewriter.replaceOpWithNewOp<mlir::LLVM::SwitchOp>(
- select, selector,
- /*defaultDestination=*/defaultDestination,
- /*defaultOperands=*/defaultOperands,
- /*caseValues=*/rewriter.getI64VectorAttr(caseValues),
- /*caseDestinations=*/destinations,
- /*caseOperands=*/destinationsOperands,
- /*branchWeights=*/llvm::ArrayRef<std::int32_t>());
- return mlir::success();
- }
-};
-/// conversion of fir::SelectOp to an if-then-else ladder
-struct SelectOpConversion : public SelectOpConversionBase<fir::SelectOp> {
- using SelectOpConversionBase::SelectOpConversionBase;
-
llvm::LogicalResult
- matchAndRewrite(fir::SelectOp op, OpAdaptor adaptor,
- mlir::ConversionPatternRewriter &rewriter) const override {
- return this->selectMatchAndRewrite(op, adaptor, rewriter);
- }
-};
-
-/// conversion of fir::SelectRankOp to an if-then-else ladder
-struct SelectRankOpConversion
- : public SelectOpConversionBase<fir::SelectRankOp> {
- using SelectOpConversionBase::SelectOpConversionBase;
-
- llvm::LogicalResult
- matchAndRewrite(fir::SelectRankOp op, OpAdaptor adaptor,
- mlir::ConversionPatternRewriter &rewriter) const override {
- return this->selectMatchAndRewrite(op, adaptor, rewriter);
- }
-};
-
-/// Lower `fir.select_type` to LLVM IR dialect.
-struct SelectTypeOpConversion : public fir::FIROpConversion<fir::SelectTypeOp> {
- using FIROpConversion::FIROpConversion;
-
- llvm::LogicalResult
- matchAndRewrite(fir::SelectTypeOp select, OpAdaptor adaptor,
- mlir::ConversionPatternRewriter &rewriter) const override {
- mlir::emitError(select.getLoc(),
- "fir.select_type should have already been converted");
+ matchAndRewrite(OP op, typename OP::Adaptor /*adaptor*/,
+ mlir::ConversionPatternRewriter & /*rewriter*/) const override {
+ op.emitOpError("should have already been converted");
return mlir::failure();
}
};
+using SelectOpConversion =
+ SelectShouldHaveBeenConvertedStub<fir::SelectOp>;
+using SelectCaseOpConversion =
+ SelectShouldHaveBeenConvertedStub<fir::SelectCaseOp>;
+using SelectRankOpConversion =
+ SelectShouldHaveBeenConvertedStub<fir::SelectRankOp>;
+using SelectTypeOpConversion =
+ SelectShouldHaveBeenConvertedStub<fir::SelectTypeOp>;
+
/// `fir.store` --> `llvm.store`
struct StoreOpConversion : public fir::FIROpConversion<fir::StoreOp> {
using FIROpConversion::FIROpConversion;
diff --git a/flang/test/Fir/Todo/select_case_with_character.fir b/flang/test/Fir/Todo/select_case_with_character.fir
index bddcb1e18a1b3..9d3231e74df7a 100644
--- a/flang/test/Fir/Todo/select_case_with_character.fir
+++ b/flang/test/Fir/Todo/select_case_with_character.fir
@@ -1,4 +1,4 @@
-// RUN: %not_todo_cmd fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s 2>&1 | FileCheck %s
+// RUN: %not_todo_cmd fir-opt --fir-select-ops-conversion %s 2>&1 | FileCheck %s
// Test `fir.select_case` conversion to llvm with character type.
// Not implemented yet.
diff --git a/flang/test/Fir/convert-to-llvm-invalid.fir b/flang/test/Fir/convert-to-llvm-invalid.fir
index b0c66e283bf5a..c7249eed0b4b3 100644
--- a/flang/test/Fir/convert-to-llvm-invalid.fir
+++ b/flang/test/Fir/convert-to-llvm-invalid.fir
@@ -27,13 +27,50 @@ func.func @shift_not_dead(%arg0: !fir.box<!fir.array<?xf32>>, %i: index) {
// -----
-// Test `fir.select_type` conversion to llvm.
-// Should have been converted.
+// The four fir.select* terminators are lowered to cf.* earlier in the
+// pipeline (--fir-select-ops-conversion for select/select_case/select_rank,
+// --fir-polymorphic-op for select_type). Any that reach FIR-to-LLVM hit a
+// defensive stub that emits a clear "should have already been converted"
+// diagnostic.
-func.func @bar_select_type(%arg : !fir.class<!fir.type<derivedst{a:f32}>>) -> i32 {
+func.func @select_should_have_been_converted(%arg: i32) -> i32 {
+ %0 = arith.constant 1 : i32
+ // expected-error at +2{{'fir.select' op should have already been converted}}
+ // expected-error at +1{{failed to legalize operation 'fir.select'}}
+ fir.select %arg : i32 [1, ^bb1(%0:i32), unit, ^bb1(%0:i32)]
+^bb1(%a: i32):
+ return %a : i32
+}
+
+// -----
+
+func.func @select_case_should_have_been_converted(%arg: !fir.ref<i32>) -> i32 {
+ %v = fir.load %arg : !fir.ref<i32>
+ %c1 = arith.constant 1 : i32
+ // expected-error at +2{{'fir.select_case' op should have already been converted}}
+ // expected-error at +1{{failed to legalize operation 'fir.select_case'}}
+ fir.select_case %v : i32 [#fir.point, %c1, ^bb1, unit, ^bb1]
+^bb1:
+ return %v : i32
+}
+
+// -----
+
+func.func @select_rank_should_have_been_converted(%arg: i32) -> i32 {
+ %0 = arith.constant 1 : i32
+ // expected-error at +2{{'fir.select_rank' op should have already been converted}}
+ // expected-error at +1{{failed to legalize operation 'fir.select_rank'}}
+ fir.select_rank %arg : i32 [1, ^bb1(%0:i32), unit, ^bb1(%0:i32)]
+^bb1(%a: i32):
+ return %a : i32
+}
+
+// -----
+
+func.func @select_type_should_have_been_converted(%arg: !fir.class<!fir.type<derivedst{a:f32}>>) -> i32 {
%0 = arith.constant 1 : i32
%2 = arith.constant 3 : i32
- // expected-error at +2{{fir.select_type should have already been converted}}
+ // expected-error at +2{{'fir.select_type' op should have already been converted}}
// expected-error at +1{{failed to legalize operation 'fir.select_type'}}
fir.select_type %arg : !fir.class<!fir.type<derivedst{a:f32}>> [
#fir.type_is<!fir.int<4>>,^bb1(%0:i32),
diff --git a/flang/test/Fir/convert-to-llvm.fir b/flang/test/Fir/convert-to-llvm.fir
index 8d67b43ffd338..6af50b963130e 100644
--- a/flang/test/Fir/convert-to-llvm.fir
+++ b/flang/test/Fir/convert-to-llvm.fir
@@ -326,98 +326,6 @@ func.func @test_unreachable() {
// -----
-// Test `fir.select` operation conversion pattern.
-// Check that the if-then-else ladder is correctly constructed and that we
-// branch to the correct block.
-
-func.func @select(%arg : index, %arg2 : i32) -> i32 {
- %0 = arith.constant 1 : i32
- %1 = arith.constant 2 : i32
- %2 = arith.constant 3 : i32
- %3 = arith.constant 4 : i32
- fir.select %arg:index [ 1, ^bb1(%0:i32),
- 2, ^bb2(%2,%arg,%arg2:i32,index,i32),
- 3, ^bb3(%arg2,%2:i32,i32),
- 4, ^bb4(%1:i32),
- unit, ^bb5 ]
- ^bb1(%a : i32) :
- return %a : i32
- ^bb2(%b : i32, %b2 : index, %b3:i32) :
- %castidx = arith.index_cast %b2 : index to i32
- %4 = arith.addi %b, %castidx : i32
- %5 = arith.addi %4, %b3 : i32
- return %5 : i32
- ^bb3(%c:i32, %c2:i32) :
- %6 = arith.addi %c, %c2 : i32
- return %6 : i32
- ^bb4(%d : i32) :
- return %d : i32
- ^bb5 :
- %zero = arith.constant 0 : i32
- return %zero : i32
-}
-
-// CHECK-LABEL: func @select(
-// CHECK-SAME: %[[SELECTVALUE:.*]]: [[IDX:.*]],
-// CHECK-SAME: %[[ARG1:.*]]: i32)
-// CHECK: %[[C0:.*]] = llvm.mlir.constant(1 : i32) : i32
-// CHECK: %[[C1:.*]] = llvm.mlir.constant(2 : i32) : i32
-// CHECK: %[[C2:.*]] = llvm.mlir.constant(3 : i32) : i32
-// CHECK: llvm.switch %[[SELECTVALUE]] : i64, ^bb5 [
-// CHECK: 1: ^bb1(%[[C0]] : i32),
-// CHECK: 2: ^bb2(%[[C2]], %[[SELECTVALUE]], %[[ARG1]] : i32, [[IDX]], i32),
-// CHECK: 3: ^bb3(%[[ARG1]], %[[C2]] : i32, i32),
-// CHECK: 4: ^bb4(%[[C1]] : i32)
-// CHECK: ]
-
-// -----
-
-// Test `fir.select_rank` operation conversion pattern.
-// Check that the if-then-else ladder is correctly constructed and that we
-// branch to the correct block.
-
-func.func @select_rank(%arg : i32, %arg2 : i32) -> i32 {
- %0 = arith.constant 1 : i32
- %1 = arith.constant 2 : i32
- %2 = arith.constant 3 : i32
- %3 = arith.constant 4 : i32
- fir.select_rank %arg:i32 [ 1, ^bb1(%0:i32),
- 2, ^bb2(%2,%arg,%arg2:i32,i32,i32),
- 3, ^bb3(%arg2,%2:i32,i32),
- 4, ^bb4(%1:i32),
- unit, ^bb5 ]
- ^bb1(%a : i32) :
- return %a : i32
- ^bb2(%b : i32, %b2 : i32, %b3:i32) :
- %4 = arith.addi %b, %b2 : i32
- %5 = arith.addi %4, %b3 : i32
- return %5 : i32
- ^bb3(%c:i32, %c2:i32) :
- %6 = arith.addi %c, %c2 : i32
- return %6 : i32
- ^bb4(%d : i32) :
- return %d : i32
- ^bb5 :
- %zero = arith.constant 0 : i32
- return %zero : i32
-}
-
-// CHECK-LABEL: func @select_rank(
-// CHECK-SAME: %[[SELECTVALUE:.*]]: i32,
-// CHECK-SAME: %[[ARG1:.*]]: i32)
-// CHECK: %[[C0:.*]] = llvm.mlir.constant(1 : i32) : i32
-// CHECK: %[[C1:.*]] = llvm.mlir.constant(2 : i32) : i32
-// CHECK: %[[C2:.*]] = llvm.mlir.constant(3 : i32) : i32
-// CHECK: %[[SELECTOR:.*]] = llvm.sext %[[SELECTVALUE]] : i{{.*}} to i64
-// CHECK: llvm.switch %[[SELECTOR]] : i64, ^bb5 [
-// CHECK: ...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/212978
More information about the llvm-branch-commits
mailing list