[PATCH] D148930: [MemRefToLLVM] Fix the lowering of memref.assume_alignment

[Mahesh Ravishankar via Phabricator via llvm-commits]

mravishankar added inline comments.


================
Comment at: mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp:372
     Value ptrValue = rewriter.create<LLVM::PtrToIntOp>(loc, intPtrType, ptr);
     rewriter.create<LLVM::AssumeOp>(
         loc, rewriter.create<LLVM::ICmpOp>(
----------------
qcolombet wrote:
> I was talking with @mravishankar offline and he pointed out that if we are aligned at (alignPtr + offset) then alignPtr must be aligned at least with the same alignment.
> I.e., we could issue two assumes here:
> - one for the aligned base pointer.
> - one for the aligned base pointer + offset.
> 
> It sounded like useful information to lower.
> 
> What do you guys think?
To clarify this a bit. If you need an `basePtr + offset` to be aligned to a value v, you need both `basePtr` and `offset` to be aligned to `v`. Without that you cannot guarantee that `basePtr + offset` is always aligned to `v` for all values of `basePtr`. 
This is the reverse logic though. Here it is saying `basePtr + offset` is aligned, but I am not sure if can always assume that `basePtr` is aligned. Maybe we skip it for now... In most common case though, `basePtr` is also aligned. We can let the user (in this case IREE) generate `assume` for both.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148930/new/

https://reviews.llvm.org/D148930