[PATCH] D87446: [AMDGPU] Enable scheduling around FP MODE-setting instructions

Mon Sep 14 13:54:40 PDT 2020

arsenm added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/SOPInstructions.td:869
+// register, so doesn't have unmodeled side effects.
+def S_SETREG_IMM32_B32_mode : SOPK_Pseudo <
+  "s_setreg_imm32_b32",
----------------
foad wrote:
> foad wrote:
> > arsenm wrote:
> > > I'm not sure this will be encoded correctly since it's repeating the same instruction definition and not adding the physical aliases. Can you add tests checking the encoding? I think this needs to use PseudoInstExpansion
> > > I'm not sure this will be encoded correctly since it's repeating the same instruction definition and not adding the physical aliases. Can you add tests checking the encoding? I think this needs to use PseudoInstExpansion
> > 
> > 
> > I'm not sure this will be encoded correctly since it's repeating the same instruction definition and not adding the physical aliases. Can you add tests checking the encoding?
> 
> You mean something like adding -show-mc-encoding to test/CodeGen/AMDGPU/llvm.amdgcn.s.setreg.ll ?
> 
> > I think this needs to use PseudoInstExpansion
> 
> I'm not sure how to do that, because at the place where I want to define S_SETREG_IMM32_B32_mode, I don't know what real instruction it should expand to (S_SETREG_B32_gfx6_gfx7 vs S_SETREG_B32_vi vs S_SETREG_B32_gfx10). Is there an example I could follow?
Yes.

Oh right, this mechanism doesn't have a way handle multiple alternative encodings. I thought this wasn't an issue for scalar instructions, which I thought had a stable encoding. You could hack around this in emitInstruction. I do think we need better infrastructure for dealing with this problem though.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87446