[PATCH] D63494: [AMDGPU] Fix for branch offset hardware workaround

Thu Jun 20 07:30:00 PDT 2019

rampitec added inline comments.

================
Comment at: lib/Target/AMDGPU/SOPInstructions.td:1011-1018
+def S_CBRANCH_SCC0_64 : SOPP64 <
+  0x00000004, (ins sopp_brtarget:$simm16),
+  "s_cbranch_scc0 $simm16"
+>;
+def S_CBRANCH_SCC1_64 : SOPP64 <
+  0x00000005, (ins sopp_brtarget:$simm16),
+  "s_cbranch_scc1 $simm16"
----------------
rtaylor wrote:
> rtaylor wrote:
> > arsenm wrote:
> > > If the opcodes end up unavoidable, the class should be fixed to generate the branch and the dummy at the same time, rather than requiring repeating each definition with the opcode value
> > I can have a look at doing this.
> I don't think _Real is the best naming since there isn't a corresponding Pseudo but I had no clue what to call this multiclass so if anyone has a suggestion, I'm all ears.
_With_Relaxation?

================
Comment at: lib/Target/AMDGPU/SOPInstructions.td:974
+  def "" : SOPP<op, ins, asm, pattern>;
+  def _PAD_S_NOP : SOPP64<op, ins, asm, pattern>;
+}
----------------
rtaylor wrote:
> rampitec wrote:
> > Can yo use InstrMapping mapping here to avoid switches?
> Do you feel that this is worth it in this case? Seems overly complicated for a very specific use case but if it's the standard to go in this direction than ok. 
You just get rid of one switch, so it is not that big deal now.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63494