[PATCH] D64393: [AMDGPU] Fix DPP combiner check for exec modification

[Jay Foad via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:6159
+    }
+
     if (I->modifiesRegister(AMDGPU::EXEC, TRI))
----------------
vpykhtin wrote:
> foad wrote:
> > arsenm wrote:
> > > foad wrote:
> > > > vpykhtin wrote:
> > > > > foad wrote:
> > > > > > vpykhtin wrote:
> > > > > > > vpykhtin wrote:
> > > > > > > > Now I recall why I used the set originally - to detect use instruction fast. Matt, is there other way to detect whether the instruction use a register without scanning its operands?
> > > > > > > and without scanning whether an instruction is in the use list for a register.
> > > > > > We have to scan the operands of every instruction anyway, in order to test `I->modifiesRegister(AMDGPU::EXEC, TRI)`. Your original code did that too.
> > > > > Good point. BTW you can probably use readsRegister instead of the loop.
> > > > `readsRegister` doesn't appear to ignore debug operands.
> > > debug uses should only appear on the debug pseudo-instructions, which you already skipped with the ->isDebugInstr check
> > OK, if that is guaranteed (I wasn't sure) I could remove the `!Op.isDebug()` test. Do you want me to? But I still can't use `readsRegister` because I need to decrement `NumUse` multiple times if there are multiple operands that read the register.
> > 
> > I could also fold the check for modifying EXEC into the same loop, so that we don't have to do two separate loops over the operands of each instruction. But I don't know if it's worth it.
> You can calculate NumUse as a number of use_nodbg_instructions in the first loop
We've discussed that before. It only works if `use_nodbg_instructions` doesn't repeat instructions, which is only true if multiple uses by the same instruction are adjacent in the use list, which I'm not sure is necessarily true.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64393