[PATCH] D143134: [AMDGPU][SDAG] attempt to custom legalize uaddo/usubo for long operands

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 2 03:09:47 PST 2023


arsenm added a comment.

Are there any new tests that demonstrate what you are trying to do?



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:893-894
+
+  //trying to avoid suboptimal selection pattern when the carry user has 
+  //already been selected to VALU
+  for (SDNode::use_iterator UI = N->use_begin(), E = N->use_end(); UI != E;
----------------
Spaces after // and capitalize 




================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:5436
 
+SDValue SITargetLowering::lowerUADDOSUBBO(SDValue Op, SelectionDAG &DAG) const {
+  SDValue Src1 = Op->getOperand(0);
----------------
I don't see the point of this, you're just doing what the default legalization should do in 1 step instead of 2


================
Comment at: llvm/test/CodeGen/AMDGPU/carryout-selection.ll:197
-; GFX11: v_add_co_ci_u32_e64 v{{[0-9]+}}, null, s{{[0-9]+}}, 0, [[CARRY]]
+; GFX11: v_add_co_ci_u32_e64 v{{[0-9]+}}, [[CARRY]], s{{[0-9]+}}, 0, [[CARRY]]
 define amdgpu_kernel void @vuaddo64(ptr addrspace(1) %out, ptr addrspace(1) %carryout, i64 %a) #0 {
   %tid = call i32 @llvm.amdgcn.workitem.id.x()
----------------
In a precommit can you convert this to generated checks? It's hard to see what's going on here


================
Comment at: llvm/test/CodeGen/AMDGPU/udiv64.ll:352-354
+; GCN-IR-NEXT:    s_mov_b64 s[8:9], 0
+; GCN-IR-NEXT:    s_and_saveexec_b64 s[10:11], s[6:7]
+; GCN-IR-NEXT:    s_xor_b64 s[6:7], exec, s[10:11]
----------------
This is a regression


================
Comment at: llvm/test/CodeGen/AMDGPU/usubsat.ll:684-687
+; GFX6-NEXT:    v_sub_i32_e32 v0, vcc, v0, v2
+; GFX6-NEXT:    v_subb_u32_e32 v1, vcc, v1, v3, vcc
+; GFX6-NEXT:    v_cndmask_b32_e64 v0, v0, 0, vcc
+; GFX6-NEXT:    v_cndmask_b32_e64 v1, v1, 0, vcc
----------------
This is an improvement


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143134



More information about the llvm-commits mailing list