[mlir] disallow side-effecting ops in llvm.mlir.global

[ftynse]

Summary

The llvm.mlir.global operation accepts a region as initializer. This region
corresponds to an LLVM IR constant expression and therefore should not accept
operations with side effects. Add a corresponding verifier.

[mehdi_amini]

corresponds to an LLVM IR complex expression

Did you mean a "constant expression"?

[bondhugula]

LGTM. Two minor comments besides the typo in the commit summary Mehdi caught.

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

1805

expressions -> initializers?

1805

Print op as well?

-> "op with side-effect found in global initializer:\n" << op

[ftynse]

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

1805

Current op is automatically printed as a note, in addition to the caret pointer.

[ftynse]

Review

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

if (Block *b = getInitializerBlock()) {
     ReturnOp ret = cast<ReturnOp>(b->getTerminator());
     if (ret.operand_type_begin() == ret.operand_type_end())
       return emitOpError("initializer region cannot return void");
     if (*ret.operand_type_begin() != getType())
       return emitOpError("initializer region type ")
              << *ret.operand_type_begin() << " does not match global type "
              << getType();
 
+    for (Operation &op : *b) {
+      auto iface = dyn_cast<MemoryEffectOpInterface>(op);
+      if (!iface || !iface.hasNoEffect())
+        return op.emitError()
+               << "ops with side effects not allowed in global initializers";
+    }
+
     if (getValueOrNull())
       return emitOpError("cannot have both initializer value and region");
   }
 
   if (getLinkage() == Linkage::Common) {
     if (Attribute value = getValueOrNull()) {
       if (!isZeroAttribute(value)) {
         return emitOpError()

mlir/test/Dialect/LLVMIR/invalid.mlir

func @gep_out_of_bounds(%ptr: !llvm.ptr<struct<(i32, struct<(i32, f32)>)>>, %idx: i64) {
   // expected-error @below {{index 2 indexing a struct is out of bounds}}
   llvm.getelementptr %ptr[%idx, 1, 3] : (!llvm.ptr<struct<(i32, struct<(i32, f32)>)>>, i64) -> !llvm.ptr<i32>
   return
 }
 
 // -----
 
 func @non_splat_shuffle_on_scalable_vector(%arg0: vector<[4]xf32>) {
-  // expected-error@+1 {{expected a splat operation for scalable vectors}}
+  // expected-error@below {{expected a splat operation for scalable vectors}}
   %0 = llvm.shufflevector %arg0, %arg0 [0 : i32, 0 : i32, 0 : i32, 1 : i32] : vector<[4]xf32>, vector<[4]xf32>
   return
-}
+}
+
+// -----
+
+llvm.mlir.global internal @side_effecting_global() : !llvm.struct<(i8)> {
+  %0 = llvm.mlir.constant(1 : i64) : i64
+  // expected-error@below {{ops with side effects not allowed in global initializers}}
+  %1 = llvm.alloca %0 x !llvm.struct<(i8)> : (i64) -> !llvm.ptr<struct<(i8)>>
+  %2 = llvm.load %1 : !llvm.ptr<struct<(i8)>>
+  llvm.return %2 : !llvm.struct<(i8)>
+}