[Mlir-commits] [mlir] [mlir][llvm] Use zeroinitializer for TargetExtType (PR #66510)

[Lukas Sommer]

================
@@ -2554,16 +2564,7 @@ LogicalResult LLVM::ConstantOp::verify() {
     return success();
   }
   if (auto targetExtType = dyn_cast<LLVMTargetExtType>(getType())) {
-    if (!targetExtType.hasProperty(LLVM::LLVMTargetExtType::HasZeroInit))
-      return emitOpError()
-             << "target extension type does not support zero-initializer";
-    // Only a single, zero integer attribute (=zeroinitializer) is allowed for a
-    // global value with TargetExtType.
-    if (!isa<IntegerAttr>(getValue()) || !isZeroAttribute(getValue()))
-      return emitOpError()
-             << "only zero-initializer allowed for target extension types";
-
-    return success();
+    return emitOpError() << "target extension type does not support constants.";
----------------
sommerlukas wrote:

I changed the error message as you proposed. 

Using the fall-through wouldn't work without modification, because we wouldn't be catching things like this anymore:
```
%0 = llvm.mlir.constant(0 : index) : !llvm.target<"spirv.Event">
```

Modifying the if below would allow to fall-though, but I think the error message in that case does not capture the intention:
```
"only supports integer, float, string or elements attributes"
```
The attribute itself isn't the problem, it's more that constant operations do not support the type at all.

Therefore, I've kept the separate if for the target extension type and only changed the error message as proposed.

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