[PATCH] D92999: [amdgpu] Enhance load widening in the constant address space.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:7985-7986
+    int64_t Offset = 0;
+    const Value *Base =
+        GetPointerBaseWithConstantOffset(P, Offset, DAG.getDataLayout());
+    KnownBits Known = computeKnownBits(Base, DAG.getDataLayout());
----------------
arsenm wrote:
> hliao wrote:
> > arsenm wrote:
> > > I'm not 100% comfortable relying on the IR value here. Can you just use the DAG known bits?
> > Here's the tradeoff I have to make based on the current alias interface in the backend. The alias checking in MI has the assumption that the `MachineMemOperand` offset should never be any negative offsets. But, the transformation here needs to rebase the pointer by a negative one to be DWORD-aligned. Without breaking the assumption so far, we need to adjust IR mapping to ensure the correctness of alias checking.
> How is that assumed? That sounds broken
Needs a comment explaining why this is looking at the IR value. Technically we could just drop the reference (since these are known to be invariant-ish loads, I'm not sure we get much out of the aliasing information)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92999