[PATCH] D122777: [X86] Improve x86-partial-reduction to support abs intrinsic

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/X86/X86PartialReduction.cpp:225
 
-  // Operand should be a select.
-  auto *SI = dyn_cast<SelectInst>(Op);
-  if (!SI)
-    return false;
-
-  // Select needs to implement absolute value.
   Value *LHS, *RHS;
+  if (match(Op, PatternMatch::m_Intrinsic<Intrinsic::abs>()))
----------------
Sink RHS into the `else` so it doens't get accidentally used.


================
Comment at: llvm/lib/Target/X86/X86PartialReduction.cpp:226
   Value *LHS, *RHS;
-  auto SPR = matchSelectPattern(SI, LHS, RHS);
-  if (SPR.Flavor != SPF_ABS)
-    return false;
+  if (match(Op, PatternMatch::m_Intrinsic<Intrinsic::abs>()))
+    LHS = Op->getOperand(0);
----------------
Add curly braces to be consistent with the else. See the 4th example if here https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


================
Comment at: llvm/test/CodeGen/X86/sad.ll:1133
 ; This test contains two absolute difference patterns joined by an add. The result of that add is then reduced to a single element.
 ; SelectionDAGBuilder should tag the joining add as a vector reduction. We neeed to recognize that both sides can use psadbw.
 define dso_local i32 @sad_double_reduction_abs(<16 x i8>* %arg, <16 x i8>* %arg1, <16 x i8>* %arg2, <16 x i8>* %arg3) {
----------------
This part about SelectionDAGBuilder I think refers to the how we handled these cases before X86PartialReduction.cpp was added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122777