[PATCH] D127095: [llvm][CodeGen] Add a default implementation to check whether two memory accesses are trivially disjoint

[Xiaoqiang Xu via Phabricator via llvm-commits]

csstormq added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineInstr.cpp:1242
+  // disjoint.
+  if (MMOa->getAddrSpace() != MMOb->getAddrSpace()) {
+    return true;
----------------
efriedma wrote:
> Address spaces are, in general, allowed to overlap.  I think there might be a target hook to check if two address spaces are disjoint.
It makes sense.


================
Comment at: llvm/lib/CodeGen/MachineInstr.cpp:1274
+  const auto EndAddrB = StartAddrB + WidthB;
+  return std::max(StartAddrA, StartAddrB) >= std::min(EndAddrA, EndAddrB);
+}
----------------
efriedma wrote:
> Do we care if all the bits are actually address-significant?  For example, on AArch64, there's a setting to tell the processor to ignore the top 16 bits of a pointer.
> 
> I guess there are other places we do aliasing checks based on the pointer bits, so maybe this isn't a practical issue.
I didn't take this into account. Maybe it's better to provide a target hook here.

At present, I don't find that some places, especially the users of MachineInstr::mayAlias, have done similar thing as me. So it will be helpful for some targets like ours, I guess.


================
Comment at: llvm/lib/CodeGen/MachineInstr.cpp:1358
 
   return !AA->isNoAlias(
       MemoryLocation(ValA, OverlapA, UseTBAA ? MMOa->getAAInfo() : AAMDNodes()),
----------------
efriedma wrote:
> Do we reach this call isNoAlias() for two different intoptr constants?  Does it not handle the case you're trying to deal with?
> 
I migrate our backend to the latest main branch. And I tried again. I truly reach this call isNoAlias() for two different intoptr constants using gdb step by step, but it still returns false wrongly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127095