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

[Jay Foad via Phabricator via llvm-commits]

foad 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",
----------------
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




================
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:
> 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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87446