[PATCH] D54959: [X86] Add a combine for back to back VSRAI instructions

Tue Nov 27 11:53:12 PST 2018

spatel added inline comments.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:35471
+  if (Opcode == X86ISD::VSRAI && N0.getOpcode() == X86ISD::VSRAI &&
+      N0.hasOneUse()) {
+    unsigned ShiftVal2 = cast<ConstantSDNode>(N0.getOperand(1))->getZExtValue();
----------------
I don't think we should limit this with hasOneUse (we don't have that restriction in the equivalent generic DAG or IR folds). Add a test for that scenario?

================
Comment at: test/Analysis/CostModel/X86/testshiftashr.ll:264
   ; SSE2-CODEGEN-LABEL: shift4i16const
-  ; SSE2-CODEGEN: psrad $3
+  ; SSE2-CODEGEN: psrad $19

----------------
Is it weird that we're (only partially) checking codegen in a cost model test file?

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

https://reviews.llvm.org/D54959