[CIR] Fold ComplexRealOp from ComplexCreateOp

[AmrDeveloper]

Folding ComplexRealOp if the operand is ComplexCreateOp, inspired by MLIR Complex dialect

Ref:

llvm-project/mlir/lib/Dialect/Complex/IR/ComplexOps.cpp

Lines 237 to 245 in 8b65c9d

	OpFoldResult ReOp::fold(FoldAdaptor adaptor) {
	ArrayAttr arrayAttr =
	llvm::dyn_cast_if_present<ArrayAttr>(adaptor.getComplex());
	if (arrayAttr && arrayAttr.size() == 2)
	return arrayAttr[0];
	if (auto createOp = getOperand().getDefiningOp<CreateOp>())
	return createOp.getOperand(0);
	return {};
	}

#141365

[llvmbot]

@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Amr Hesham (AmrDeveloper)

Changes

Folding ComplexRealOp if the operand is ComplexCreateOp, inspired by MLIR Complex dialect

Ref:

llvm-project/mlir/lib/Dialect/Complex/IR/ComplexOps.cpp

Lines 237 to 245 in 8b65c9d

	OpFoldResult ReOp::fold(FoldAdaptor adaptor) {
	ArrayAttr arrayAttr =
	llvm::dyn_cast_if_present<ArrayAttr>(adaptor.getComplex());
	if (arrayAttr && arrayAttr.size() == 2)
	return arrayAttr[0];
	if (auto createOp = getOperand().getDefiningOp<CreateOp>())
	return createOp.getOperand(0);
	return {};
	}

#141365

---

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

2 Files Affected:

• (modified) clang/lib/CIR/Dialect/IR/CIRDialect.cpp (+5)
• (modified) clang/test/CIR/Transforms/complex-real-fold.cir (+17-1)

diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
index 8512b229c2663..ed70e52aaefe6 100644
--- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
@@ -2066,6 +2066,11 @@ LogicalResult cir::ComplexRealOp::verify() {
 }
 
 OpFoldResult cir::ComplexRealOp::fold(FoldAdaptor adaptor) {
+  if (auto complexCreateOp = dyn_cast_or_null<cir::ComplexCreateOp>(
+          getOperand().getDefiningOp())) {
+    return complexCreateOp.getOperand(0);
+  }
+
   auto complex =
       mlir::cast_if_present<cir::ConstComplexAttr>(adaptor.getOperand());
   return complex ? complex.getReal() : nullptr;
diff --git a/clang/test/CIR/Transforms/complex-real-fold.cir b/clang/test/CIR/Transforms/complex-real-fold.cir
index 1cab9be616af0..630dd679f67af 100644
--- a/clang/test/CIR/Transforms/complex-real-fold.cir
+++ b/clang/test/CIR/Transforms/complex-real-fold.cir
@@ -1,4 +1,4 @@
-// RUN: cir-opt %s -cir-canonicalize -o - | FileCheck %s
+// RUN: cir-opt %s -cir-canonicalize  -o - -split-input-file | FileCheck %s
 
 !s32i = !cir.int<s, 32>
 
@@ -21,3 +21,19 @@ module {
   // CHECK: }
 
 }
+
+// -----
+
+!s32i = !cir.int<s, 32>
+
+module {
+  cir.func dso_local @fold_complex_real_from_create_test(%arg0: !s32i, %arg1: !s32i) -> !s32i {
+    %0 = cir.complex.create %arg0, %arg1 : !s32i -> !cir.complex<!s32i>
+    %1 = cir.complex.real %0 : !cir.complex<!s32i> -> !s32i
+    cir.return %1 : !s32i
+  }
+
+  // CHECK: cir.func dso_local @fold_complex_real_from_create_test(%[[ARG_0:.*]]: !s32i, %[[ARG_1:.*]]: !s32i) -> !s32i {
+  // CHECK:   cir.return %[[ARG_0]] : !s32i
+  // CHECK: }
+}

[andykaylor]

I'm not sure we want to do this as part of our general canonicalization. It's a good optimization, but I don't think this kind of thing should be happening before the optimization phase. That may be true of most of our folders.

I suppose this is a broad design decision that needs to be made. The way I have viewed clang in the past is that the clang component creates IR that faithfully models the semantics of the source code, and all optimizations should be left to the optimizer. That's a bit different than the general MLIR view, but I think that's because MLIR has mostly developed in a different environment from traditional compiler's like clang.

I don't know if that means we shouldn't be running cir-canonicalize unconditionally, or if transformations like this should be implemented in cir-simplify.

[xlauko]

I don't see real use of not allowing this by default, as this is essentially identity folding, not really optimization?
And applying folds is way cheaper then rewrite patterns later.

Though yes, I agree we should have discussion where is the borderline, for instance UnaryOp::fold is doing more simplification/rewrite than this from my point of view.

[xlauko]

Suggested change

	// RUN: cir-opt %s -cir-canonicalize -o - -split-input-file | FileCheck %s
	// RUN: cir-opt %s -cir-canonicalize -split-input-file -o - | FileCheck %s

[bcardosolopes]

if transformations like this should be implemented in cir-simplify

For this specific one I agree with @xlauko, it could have already come out of CIRGen like this without hurting source fidelity. For anything slightly more, cir-simplify should be used. We just went through a similar one with vector (in which the folding to constant vector (from a splat op) can actually increase code size), so we moved it to cir-simplify (and should probably take opt level in consideration in the future).

Though yes, I agree we should have discussion where is the borderline, for instance UnaryOp::fold is doing more simplification/rewrite than this from my point of view.

Yea, unary and a few others. We already encode the expectation as part of these passes documentation/description, but some are subtle enough that review time is probably the best we can do?

[bcardosolopes]

Curly braces not needed