[PATCH] D154083: [AMDGPU] Rematerialize scalar loads

[Piotr Sobczak via Phabricator via llvm-commits]

piotr added a comment.

In D154083#4490691 <https://reviews.llvm.org/D154083#4490691>, @arsenm wrote:

> Could really use a MIR test that shows this. Also would be nice to have some evil cases, where the result register is tied to the input pointer register

This patch is now based on a test update (https://reviews.llvm.org/D154816), where I am also adding a new test that exercises the shrinking - test_remat_s_load_dword_immx16_subreg.

Can you describe the evil case(s) in more detail? Do you mean S_LOAD_DWORDX16_IMM with tied-def, or something else?



================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:111
+  if (isVOP1(MI) || isVOP2(MI) || isVOP3(MI) || isSDWA(MI) || isSALU(MI) ||
+      (isSMRD(MI) && MI.isDereferenceableInvariantLoad())) {
     // Normally VALU use of exec would block the rematerialization, but that
----------------
arsenm wrote:
> arsenm wrote:
> > piotr wrote:
> > > arsenm wrote:
> > > > piotr wrote:
> > > > > Ironically, the addition of the check `isDereferenceableInvariantLoad` makes the examples that drive this work not handled, as the S_LOADs are not marked as `dereferenceable` at the moment (but that is another issue).
> > > > Don't really need the isSMRD with the isDereferenceableInvariantLoad. We should also be able to handle any load
> > > That was my intuition as well, but I added this extra check for these reasons:
> > > - the negative tests in remat-smrd.mir contain cases where the loads would be rematted otherwise e.g. test_no_remat_s_load_dword_not_dereferenceable
> > > - the generic hook that we bypass has a similar condition for loads (https://github.com/llvm/llvm-project/blob/c304be7cfdd2261811671feb252e31222365b475/llvm/lib/CodeGen/TargetInstrInfo.cpp#L1103)
> > > 
> > > Maybe checking for MMO->isInvariant() is enough? And that would mean the negative test would turn to a positive one.
> > > 
> > Would rematerialization ever happen in a context where the use didn't have the same dominating conditions as at the def? My intuition is that it couldn't, and thus the dereferenceable check (including the generic one) would be too strong 
> As a pre-commit can you post a patch to weaken the generic check? Best to be on the same page as all targets for this
Ok, will take a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154083