[Mlir-commits] [mlir] [MLIR][MemRef] Complete alignment type migration in OpBuilders (PR #159449)

[llvmlistbot at llvm.org]

jiang1997 wrote:

> Hi @jiang1997 , thanks for your patience.
> 
> Can you clean up the constructors that use `valueOrOne().value()` to just use `alignment->value()`? After that I think it would be good to merge.
> 
> I think this is a good change but it is not the final one. There is one after this one which I wanted to verify how it would look like after this change was applied. The next change would be to add an [OpInterface](https://mlir.llvm.org/docs/Interfaces/#attributeoperationtype-interfaces) that defines a default method (possibly called) `getMaybeAlign` which would return the alignment as an `llvm::MaybeAlign`. With this, we could now use it with the constructors that receive an `llvm::MaybeAlign`. E.g.,
> 
> ```c++
>           [&](OpBuilder &builder, Location loc) {
>             auto loadedValue = memref::LoadOp::create(
>                 builder, loc, base, indices, /*nontemporal=*/false,
>                 maskedLoad.getMaybeAlign()):
> ```
> 
> Let me know if you want to work on that one too, but no pressure. I think this last one may receive more reviewer comments.

Yes, I know this isn’t the final one— the original issue also mentioned adding the new interface. I’m willing to keep contributing.

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