[PATCH] D68893: AMDGPU: Split flat offsets that don't fit in DAG

Tue Oct 15 13:19:11 PDT 2019

arsenm marked an inline comment as done.
arsenm added a comment.

In D68893#1708171 <https://reviews.llvm.org/D68893#1708171>, @nhaehnle wrote:

> Mostly LGTM, but I wonder about the high level intention here. Is this intended to expose new load/store merging opportunities? If so, is there a test for this? Or is there some part of SIFoldOperands that can now be removed?

This is mostly covered already, by promote-constOffset-to-imm.ll but I did mean to add more target cases for this. The problem I was solving is that SILoadStoreOptimizer tries to do this optimization currently. When D68894 <https://reviews.llvm.org/D68894> is applied, this would break since SIFoldOperands would now shrink the add pattern it's looking for. The shrunk form would require vcc liveness tracking, so it's easier to just split the offset earlier

================
Comment at: lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:1715
+#if 0
+      // TODO: Should this try to use a scalar add pseudo?
+      SDValue AddOffset
----------------
nhaehnle wrote:
> It does seem like an annoying duplication of concerns to implement the 64-bit addition here manually.
Because we don't try to change SALU operations to VALU based on a VGPR use, this has some test consequences. Some cases are better with the scalar input but I think the VALU output gave a slightly better result

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

https://reviews.llvm.org/D68893