[PATCH] D133584: [DAGCombiner] [AMDGPU] Allow vector loads in MatchLoadCombine

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

I'm still not confident in my understanding of the various index values even with the added code comments. 
I'll try to step through some of these tests in the debugger to get a better idea, but it would be good if another reviewer can have a look too for a second opinion.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8349-8350
+
+    // Add the VectorOffset (if any) to the offset of Ptr
+    Ptr.setOffset(Ptr.getOffset() + P->VectorOffset);
+
----------------
spatel wrote:
> Do we have a test where VectorOffset is non-zero? If not, please add one (add baseline tests as needed, no pre-commit review is necessary).
This is marked as done, but I can't tell from just looking at the tests which one exercises that path. Add an example/comment here or next to a test, so it's clearer what this looks like?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133584