[PATCH] D64911: [AMDGPU] Extend the SI Load/Store optimizer

[Piotr Sobczak via Phabricator via llvm-commits]

piotr marked an inline comment as done.
piotr added inline comments.


================
Comment at: lib/Target/AMDGPU/SILoadStoreOptimizer.cpp:322-325
+      if (AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::vaddr) == -1)
+        return UNKNOWN;
+      if (!TII.get(Opc).mayLoad() || TII.isGather4(Opc))
+        return UNKNOWN;
----------------
nhaehnle wrote:
> piotr wrote:
> > nhaehnle wrote:
> > > This should probably check mayStore instead of mayLoad: we want to exclude both stores and atomics.
> > > 
> > > You could also move the check for TFE and LWE to here.
> > Good point about atomics, I added the condition to bail out on mayStore()). I am keeping !mayLoad() to avoid merging IMAGE_GET_RESINFO.
> > 
> > For TFE/LWE I would like to keep the checks where they are, because I dislike extending getInstClass() with Instruction argument, and it would be necessary to query the actual value of TFE/LWE.
> Can IMAGE_GET_RESINFO not be merged? I would kind of expect it can...
> 
> Fair point about TFE/LWE.
Yes, ideally we would like to combine them too, but currently it would not work, because the pass makes assumptions that the instruction to be merged is either a load or store (assertion thrown otherwise) . I will add a TODO in the code to make it clear that IMAGE_GET_RESINFO is not handled.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64911