[PATCH] D55071: [AMDGPU] Split 64-Bit XNOR to 64-Bit NOT/XOR

[Graham Sellers via Phabricator via llvm-commits]

grahamsellers marked 3 inline comments as done.
grahamsellers added inline comments.


================
Comment at: lib/Target/AMDGPU/SIInstrInfo.cpp:4908
+  MachineOperand &Src1 = Inst.getOperand(2);
+  DebugLoc DL = Inst.getDebugLoc();
+
----------------
arsenm wrote:
> const reference
I stole this from elsewhere in this file. I went ahead and fixed other places where this was not const reference.


================
Comment at: test/CodeGen/AMDGPU/xnor.ll:128
+  %r0.val = xor i64 %xor, -1
+  store i64 %r0.val, i64 addrspace(1)* %r0
+  ret void
----------------
arsenm wrote:
> It would be good if you can include some other use that requires SALU->VALU changes to stress that the wordlist insert happened
Not sure I follow. The new split turns S_XNOR_B64 into S_NOT_B64 followed by S_XOR_B64, adding the S_XOR_B64 to the worklist. The only way the two V_XOR_B32 instructions can be generated is when SIInstrInfo::moveToVALU iterates again, converting S_XOR_B64 -> 2 x S_XOR_B32, which re-adds to the worklist, causing moveToVALU to turn S_XOR_B32 to V_XOR_B32_E32 (which the test expects). If the worklist insert wasn't happening, none of these tests would work.

What am I missing?


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

https://reviews.llvm.org/D55071