Skip to content

[mlir][emitc] Only mark operator with fundamental type have no side effect #144990

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jacquesguan
Copy link
Contributor

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Jun 20, 2025

@llvm/pr-subscribers-mlir-emitc

@llvm/pr-subscribers-mlir

Author: Jianjian Guan (jacquesguan)

Changes

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

3 Files Affected:

  • (modified) mlir/include/mlir/Dialect/EmitC/IR/EmitC.h (+3)
  • (modified) mlir/include/mlir/Dialect/EmitC/IR/EmitC.td (+17-2)
  • (modified) mlir/lib/Dialect/EmitC/IR/EmitC.cpp (+5)
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.h b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.h
index 1984ed8a7f068..eb7ddeb3bfc54 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.h
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.h
@@ -53,6 +53,9 @@ bool isPointerWideType(mlir::Type type);
 struct Placeholder {};
 using ReplacementItem = std::variant<StringRef, Placeholder>;
 
+/// Determines whether \p type is a valid fundamental C++ type in EmitC.
+bool isFundamentalType(mlir::Type type);
+
 } // namespace emitc
 } // namespace mlir
 
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
index 9ecdb74f4d82e..c66c3824bcacd 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
@@ -43,7 +43,11 @@ class EmitC_UnaryOp<string mnemonic, list<Trait> traits = []> :
 
   let extraClassDeclaration = [{
     bool hasSideEffects() {
-      return false;
+      // If operand is fundamental type, the operation is pure.
+      if (isFundamentalType(getOperand().getType())) {
+        return false;
+      }
+      return true;
     }
   }];
 }
@@ -57,7 +61,18 @@ class EmitC_BinaryOp<string mnemonic, list<Trait> traits = []> :
 
   let extraClassDeclaration = [{
     bool hasSideEffects() {
-      return false;
+      // If both operands are fundamental types, the operation is pure.
+      if (isFundamentalType(getOperand(0).getType()) 
+          && isFundamentalType(getOperand(1).getType())) {
+        return false;
+      }
+      // Pointer arithmetic operations is pure.
+      if (isa<emitc::PointerType>(getOperand(0).getType()) 
+          && (isFundamentalType(getOperand(1).getType()) ||
+              isa<emitc::PointerType>(getOperand(1).getType()))) {
+        return false;
+      }
+      return true;
     }
   }];
 }
diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index e602210c2dc6c..bb176d5fad808 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -137,6 +137,11 @@ bool mlir::emitc::isPointerWideType(Type type) {
       type);
 }
 
+bool mlir::emitc::isFundamentalType(Type type) {
+  return llvm::isa<IndexType>(type) || isPointerWideType(type) ||
+         isSupportedIntegerType(type) || isSupportedFloatType(type);
+}
+
 /// Check that the type of the initial value is compatible with the operations
 /// result type.
 static LogicalResult verifyInitializationAttribute(Operation *op,

@mgehre-amd
Copy link
Contributor

Hey, thanks for the PR. Can you please add a test case that shows the new behavior?

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.

3 participants