[PATCH] D56636: [CodeGen][X86] Expand vector USUBSAT to UMAX+SUB

Simon Pilgrim via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jan 12 04:35:04 PST 2019


RKSimon added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:334
   case ISD::ADD:
   case ISD::SUB:
   case ISD::MUL:
----------------
Add ISD::USUBSAT et al here as well?


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:5307
 
 SDValue TargetLowering::getExpandedSaturationAdditionSubtraction(
     SDNode *Node, SelectionDAG &DAG) const {
----------------
Really pedantic, but this function name is massive - why not just TargetLowering::expandAddSubSat ?


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:5317
+    // usub.sat(a, b) -> umax(a, b) - b
+    if (Opcode == ISD::USUBSAT && isOperationLegal(ISD::UMAX, VT)) {
+      SDValue Max = DAG.getNode(ISD::UMAX, dl, VT, LHS, RHS);
----------------
isOperationLegalOrCustom?


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:1224
+    if (!HasInt256)
+      setOperationAction(ISD::USUBSAT, MVT::v8i32, Custom);
 
----------------
LowerADDSAT_SUBSAT supports AVX1 splitting, so why not:
```
setOperationAction(ISD::USUBSAT, MVT::v8i32, HasInt256 ? Legal : Custom);
```

Also, v4i64  should be ok as well.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56636





More information about the llvm-commits mailing list