[PATCH] D52677: [AMDGPU] Match v_swap_b32

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPUSubtarget.h:519
+  bool hasSwap() const {
+    return getGeneration() >= AMDGPUSubtarget::GFX9;
+  }
----------------
There's a specific GFX9Insts feature (although we have some others that should be checking it instead of the generation already)


================
Comment at: lib/Target/AMDGPU/SIInstrInfo.h:234-238
+static bool instReadsReg(const MachineInstr *MI,
+                         unsigned Reg, unsigned SubReg,
+                         const SIRegisterInfo &TRI) {
+  return instAccessReg(MI->uses(), Reg, SubReg, TRI);
+}
----------------
rampitec wrote:
> arsenm wrote:
> > How is this different from MachineInstr::readsRegister? Does that just to add a subreg operand?
> Yes, reads/modifiesRegister would return true if a super-register is modified even if a particular lane of interest does not. Some of the foldings in the test would fail if I use it.
This needs a comment explaining it then


================
Comment at: lib/Target/AMDGPU/SIInstrInfo.h:286
+  MachineOperand *YTop = nullptr;
+  for (MachineOperand &Op : MRI.use_operands(T)) {
+    if (Op.getSubReg() != Tsub) {
----------------
arsenm wrote:
> use_nodbg_operands?
I still see it as use_operands?


================
Comment at: test/CodeGen/AMDGPU/v_swap_b32.mir:521
+    %1.sub0 = COPY %2.sub0
+...
+
----------------
%2 needs another use which might break


https://reviews.llvm.org/D52677