[PATCH] D37348: Implement custom lowering for ISD::CTTZ_ZERO_UNDEF and ISD::CTTZ.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2803-2806
     // If ISD::CTLZ is legal and CTPOP isn't, then do that instead.
     if (!TLI.isOperationLegalOrCustom(ISD::CTPOP, VT) &&
         TLI.isOperationLegalOrCustom(ISD::CTLZ, VT))
       return DAG.getNode(ISD::SUB, dl, VT,
----------------
OK, I see the default expansion here isn't the compare and select like I expected. Since the compare+select implementation is likely more instructions with the compare than the sub/ctpop implementation, that one should be tried first.


================
Comment at: lib/Target/AMDGPU/AMDGPUISelLowering.cpp:417
   if (Subtarget->hasFFBH())
     setOperationAction(ISD::CTLZ_ZERO_UNDEF, MVT::i32, Custom);
 
----------------
We should probably fix this at some point to be legal


================
Comment at: lib/Target/AMDGPU/AMDGPUISelLowering.cpp:1109-1110
+  case ISD::CTTZ: 
+  case ISD::CTTZ_ZERO_UNDEF:
+		return LowerCTTZ(Op, DAG);
   case ISD::CTLZ:
----------------
Also need the select with -1 optimization (and corresponding tests) as cttz


================
Comment at: lib/Target/AMDGPU/AMDGPUISelLowering.cpp:2021
+
+SDValue AMDGPUTargetLowering:: LowerCTTZ(SDValue Op, SelectionDAG &DAG) const {
+  SDLoc SL(Op);
----------------
This is mostly copy past from LowerCTLZ. These should be factored into a common helper.


================
Comment at: test/CodeGen/AMDGPU/cttz_zero_undef.ll:103
+}
+
----------------
Need i64 tests


Repository:
  rL LLVM

https://reviews.llvm.org/D37348