[PATCH] D54959: [X86] Add a combine for back to back VSRAI instructions
Sanjay Patel via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 28 06:04:57 PST 2018
spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.
LGTM. Might want to leave a 'FIXME' note on that cost model test file. Or move anything of value in there that isn't already tested in the codegen test file.
================
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();
----------------
craig.topper wrote:
> 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.
Thanks - those diffs are what I imagined. It's better to trade a move for a shift (at least on paper) and reduce the dependency chain, and that's consistent with other folds.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54959/new/
https://reviews.llvm.org/D54959
More information about the llvm-commits
mailing list