[Mlir-commits] [mlir] [mlir][vector] Add alignment attribute to `maskedload` and `maskedstore` (PR #151690)

[Erick Ochoa Lopez]

================
@@ -1729,18 +1729,18 @@ def Vector_LoadOp : Vector_Op<"load", [
                    "Value":$base,
                    "ValueRange":$indices,
                    CArg<"bool", "false">:$nontemporal,
-                   CArg<"uint64_t", "0">:$alignment), [{
+                   CArg<"llvm::Align", "llvm::Align()">:$alignment), [{
----------------
amd-eochoalo wrote:

Thanks @krzysz00 ! You are right, as it is currently implemented, `llvm::Align()` defaults to 1-byte alignment. We can change the code and use `llvm::MaybeAlign`. It would look as follows because `llvm::MaybeAlign`'s implementation is a simple wrapper around an `llvm::Align`.

```diff
diff --git a/mlir/include/mlir/Dialect/Vector/IR/VectorOps.td b/mlir/include/mlir/Dialect/Vector/IR/VectorOps.td
index 6a98024e7f22..35c9c1df4446 100644
--- a/mlir/include/mlir/Dialect/Vector/IR/VectorOps.td
+++ b/mlir/include/mlir/Dialect/Vector/IR/VectorOps.td
@@ -1729,18 +1729,18 @@ def Vector_LoadOp : Vector_Op<"load", [
                    "Value":$base,
                    "ValueRange":$indices,
                    CArg<"bool", "false">:$nontemporal,
-                   CArg<"llvm::Align", "llvm::Align()">:$alignment), [{
+                   CArg<"llvm::MaybeAlign", "llvm::MaybeAlign()">:$alignment), [{
       return build($_builder, $_state, resultType, base, indices, nontemporal,
-                   alignment != llvm::Align() ? $_builder.getI64IntegerAttr(alignment.value()) :
+                   alignment != llvm::MaybeAlign() ? $_builder.getI64IntegerAttr(alignment.valueOrOne().value()) :
                                     nullptr);
     }]>,
     OpBuilder<(ins "TypeRange":$resultTypes,
                    "Value":$base,
                    "ValueRange":$indices,
                    CArg<"bool", "false">:$nontemporal,
-                   CArg<"llvm::Align", "llvm::Align()">:$alignment), [{
+                   CArg<"llvm::MaybeAlign", "llvm::MaybeAlign()">:$alignment), [{
       return build($_builder, $_state, resultTypes, base, indices, nontemporal,
-                   alignment != llvm::Align() ? $_builder.getI64IntegerAttr(alignment.value()) :
+                   alignment != llvm::MaybeAlign() ? $_builder.getI64IntegerAttr(alignment.valueOrOne().value()) :
                                     nullptr);
     }]>
   ];
```

 Let me know what you think. I would prefer not to have an `valueOrOne().value()` call like that. I can re-implement the struct which I previously reverted. cc @kuhar , what do you think?

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