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

[Craig Topper via Phabricator via llvm-commits]

craig.topper marked 2 inline comments as done.
craig.topper 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();
----------------
spatel wrote:
> 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?
Without it we generated extra moves on some other test cases in vector-shift-ashr-sub128.ll at least on pre-AVX targets.


================
Comment at: test/Analysis/CostModel/X86/testshiftashr.ll:264
   ; SSE2-CODEGEN-LABEL: shift4i16const
-  ; SSE2-CODEGEN: psrad $3
+  ; SSE2-CODEGEN: psrad $19
 
----------------
spatel wrote:
> Is it weird that we're (only partially) checking codegen in a cost model test file?
Yeah its pretty weird. We didn't even have complete codegen test coverage of the narrow vectors until I added the sub128 tests yesterday.


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

https://reviews.llvm.org/D54959