[flang-commits] [flang] [flang] Region-based HLFIR operation for conditional expressions lowering (PR #194411)

[via flang-commits]

================
@@ -857,6 +858,159 @@ struct CharExtremumOpConversion
   }
 };
 
+struct ConditionalOpConversion
+    : public mlir::OpConversionPattern<hlfir::ConditionalOp> {
+  using mlir::OpConversionPattern<hlfir::ConditionalOp>::OpConversionPattern;
+  explicit ConditionalOpConversion(mlir::MLIRContext *ctx)
+      : mlir::OpConversionPattern<hlfir::ConditionalOp>{ctx} {
+    // This pattern recursively converts nested ConditionalOp's
+    // by cloning and then converting them, so we have to allow
+    // for recursive pattern application. The recursion is bounded
+    // by the nesting level of ConditionalOp's.
+    setHasBoundedRewriteRecursion();
+  }
+  llvm::LogicalResult
+  matchAndRewrite(hlfir::ConditionalOp condOp, OpAdaptor adaptor,
+                  mlir::ConversionPatternRewriter &rewriter) const override {
+    const mlir::Location loc{condOp->getLoc()};
+    fir::FirOpBuilder builder(rewriter, condOp.getOperation());
+    HLFIRListener listener{builder, rewriter};
+    builder.setListener(&listener);
+    // Use ExprType to ensure both branches produce identical MLIR temp types.
+    const auto exprType{
+        mlir::cast<hlfir::ExprType>(condOp.getResult().getType())};
+    const mlir::Type tempBaseType{hlfir::getVariableType(exprType)};
+
+    // Emit one branch: clone ops, create temp, assign, run deferred
+    // destroys, yield (temp, mustFree).
+    auto emitBranch = [&](mlir::Region &region) {
+      mlir::IRMapping mapper;
+      auto yield{mlir::cast<hlfir::YieldOp>(region.front().getTerminator())};
+      const mlir::Value yieldedEntity{yield.getEntity()};
+      // Try to forward the yielded entity's storage directly to avoid using a
+      // temp.
+      hlfir::AsExprOp asExprToForward;
+      if (yieldedEntity.getType() == exprType) {
+        if (auto asExpr{yieldedEntity.getDefiningOp<hlfir::AsExprOp>()};
+            asExpr && asExpr.isMove() &&
+            asExpr->getBlock() == &region.front() &&
+            asExpr.getVar().getType() == tempBaseType &&
+            llvm::all_of(yieldedEntity.getUsers(), [&](mlir::Operation *user) {
+              return user == yield.getOperation() ||
+                     mlir::isa<hlfir::DestroyOp>(user);
+            }))
+          asExprToForward = asExpr;
+      }
+      // Ops after yieldDefOp are cleanups that must be deferred until after
+      // the temp assign to avoid use-after-free.
+      mlir::Operation *const yieldDefOp{yieldedEntity.getDefiningOp()};
+      const bool hasDeferredCleanups{!asExprToForward && yieldDefOp &&
+                                     yieldDefOp->getBlock() == &region.front()};
----------------
jeanPerier wrote:

Do you have an example where this is needed? Cleanups should not affect the hlfir.expr value (except for the hlfir.destroy).

If this is indeed needed, then it would be best to modify the lowering so that the statement context clean-up is generated inside the yield cleanup region.

https://github.com/llvm/llvm-project/pull/194411