[PATCH] D78091: [AMDGPU] Enable carry out ADD/SUB operations divergence driven instruction selection.

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 15 11:30:31 PDT 2020


arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:1086
+  if (!IsVALU) {
+    for (auto Use : N->uses()) {
+      if (Use->isMachineOpcode() && TII->isVALU(Use->getMachineOpcode())) {
----------------
alex-t wrote:
> arsenm wrote:
> > Shouldn't need to scan all uses?
> I assume that the one user selected to VALU is enough to make selecting carryout to SALU impractical. Also, how would you suggest to decide about VALU/SALU basing on the quantity of VALU users? Ratio VALU/SALU? Other heuristic?
There's already  a isVGPRImm function, which should be functionally the same


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp:918-923
+  // TODO: We have to add FinalizeISel
+  // to expand V_ADD/SUB_U64_PSEUDO before SIFixupVectorISel
+  // that expects V_ADD/SUB -> A_ADDC/SUBB pairs expanded.
+  // Will be removed as soon as SIFixupVectorISel is changed
+  // to work with V_ADD/SUB_U64_PSEUDO instead.
+  addPass(&FinalizeISelID);
----------------
This is just broken. We already run it, and there shouldn't' be a reason to involve SIFixupVectorISel


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

https://reviews.llvm.org/D78091





More information about the llvm-commits mailing list