[MLIR][EmitC] Disallow to apply the op & to const

[QuqqU]

Summary

Disallow to apply the operator & (address of) to emitc.constant operations.

[QuqqU]

ping

Please take a look, and let me know if there is an issue :)

[marbre]

In D147992#4262163, @QuqqU wrote:

ping

Please take a look, and let me know if there is an issue :)

I added Simon and myself as reviewers, as we initial pushed the code and mostly maintain it. We will take a look early next week.

[simon-camp]

Thanks for the cleanup @QuqqU. This LGTM.

[QuqqU]

@marbre @simon-camp Could you mind if I ask you push it on my behalf?
I have no commit access because this is the first time.
I really appreciate!

• name : Kiung Jung
• email : [e-mail]

[marbre]

In D147992#4299008, @QuqqU wrote:

@marbre @simon-camp Could you mind if I ask you push it on my behalf?
I have no commit access because this is the first time.
I really appreciate!

Sure! I am happy to land the patch on your behalf.

[marbre]

Thanks for the patch! Tested locally, LGTM. Unfortunately, I have some problems with arcanist grabbing the complete patch. I will try to land it tomorrow.

[QuqqU]

In D147992#4299476, @marbre wrote:

Thanks for the patch! Tested locally, LGTM. Unfortunately, I have some problems with arcanist grabbing the complete patch. I will try to land it tomorrow.

I greatly appreciate for your help :>
If there is an issue(eg. metadata) on my patch, please let me know.

[marbre]

In D147992#4301154, @QuqqU wrote:

In D147992#4299476, @marbre wrote:

Thanks for the patch! Tested locally, LGTM. Unfortunately, I have some problems with arcanist grabbing the complete patch. I will try to land it tomorrow.

I greatly appreciate for your help :>
If there is an issue(eg. metadata) on my patch, please let me know.

arc patch didn't worked out so I had to apply the raw patch file. When committing your changes I accidentally modified the commit message and dropped [MLIR][EmitC]. Sorry for that. I will be more careful when landing your second patch!

[QuqqU]

In D147992#4302287, @marbre wrote:

In D147992#4301154, @QuqqU wrote:

In D147992#4299476, @marbre wrote:

Thanks for the patch! Tested locally, LGTM. Unfortunately, I have some problems with arcanist grabbing the complete patch. I will try to land it tomorrow.

I greatly appreciate for your help :>
If there is an issue(eg. metadata) on my patch, please let me know.

arc patch didn't worked out so I had to apply the raw patch file. When committing your changes I accidentally modified the commit message and dropped [MLIR][EmitC]. Sorry for that. I will be more careful when landing your second patch!

Although it is a small one, I am happy to contribute to this project.
Thanks for your kindness, and I'll make more contributions.

mlir/include/mlir/Dialect/EmitC/IR/EmitC.td

let description = [{
     %0 = emitc.apply "&"(%arg0) : (i32) -> !emitc.opaque<"int32_t*">
 
     // Generic form of the same operation.
     %0 = "emitc.apply"(%arg0) {applicableOperator = "&"}
         : (i32) -> !emitc.opaque<"int32_t*">
 
     ```
   }];
-  // TODO: Disallow to apply the operator & (address of) to emitc.constant operations.
   let arguments = (ins
     Arg<StrAttr, "the operator to apply">:$applicableOperator,
     AnyType:$operand
   );
   let results = (outs AnyType:$result);
   let assemblyFormat = [{
     $applicableOperator `(` $operand `)` attr-dict `:` functional-type($operand, results)
   }];

mlir/lib/Dialect/EmitC/IR/EmitC.cpp

LogicalResult ApplyOp::verify() {
   // Applicable operator must not be empty.
   if (applicableOperatorStr.empty())
     return emitOpError("applicable operator must not be empty");
 
   // Only `*` and `&` are supported.
   if (applicableOperatorStr != "&" && applicableOperatorStr != "*")
     return emitOpError("applicable operator is illegal");
 
+  Operation *op = getOperand().getDefiningOp();
+  if (op && dyn_cast<ConstantOp>(op))
+    return emitOpError("cannot apply to constant");
+
   return success();
 }
 
 //===----------------------------------------------------------------------===//
 // CastOp
 //===----------------------------------------------------------------------===//
 
 bool CastOp::areCastCompatible(TypeRange inputs, TypeRange outputs) {
LogicalResult emitc::ConstantOp::verify() {
   TypedAttr value = getValueAttr();
   Type type = getType();
   if (!value.getType().isa<NoneType>() && type != value.getType())
     return emitOpError() << "requires attribute's type (" << value.getType()
                          << ") to match op's return type (" << type << ")";
   return success();
 }
 
-OpFoldResult emitc::ConstantOp::fold(FoldAdaptor adaptor) {
-  return getValue();
-}
+OpFoldResult emitc::ConstantOp::fold(FoldAdaptor adaptor) { return getValue(); }
 
 //===----------------------------------------------------------------------===//
 // IncludeOp
 //===----------------------------------------------------------------------===//
 
 void IncludeOp::print(OpAsmPrinter &p) {
   bool standardInclude = getIsStandardInclude();
 

mlir/test/Dialect/EmitC/invalid_ops.mlir

 func.func @illegal_operator(%arg : i32) {
     // expected-error @+1 {{'emitc.apply' op applicable operator is illegal}}
     %2 = emitc.apply "+"(%arg) : (i32) -> !emitc.ptr<i32>
     return
 }
 
 // -----
 
+func.func @illegal_operand() {
+    %1 = "emitc.constant"(){value = 42: i32} : () -> i32
+    // expected-error @+1 {{'emitc.apply' op cannot apply to constant}}
+    %2 = emitc.apply "&"(%1) : (i32) -> !emitc.ptr<i32>
+    return
+}
+
+// -----
+
 func.func @var_attribute_return_type_1() {
     // expected-error @+1 {{'emitc.variable' op requires attribute's type ('i64') to match op's return type ('i32')}}
     %c0 = "emitc.variable"(){value = 42: i64} : () -> i32
     return
 }
 
 // -----