Make DAGCombine turn mul by power of 2 into shl for scalable vector.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td | ||
---|---|---|
783 ↗ | (On Diff #365674) | I don't think this code is tested by this patch (via fixed-length vectors or VP intrinsics) |
DAGCombine should turn mul by 2 into shl. Except that it can't for scalable vectors because isKnownToBeAPowerOfTwo doesn't work for scalable vectors. We should just make DAG combine work even if we have just have to special case SPLAT_VECTOR.
// fold (mul x, (1 << c)) -> x << c if (isConstantOrConstantVector(N1, /*NoOpaques*/ true) && DAG.isKnownToBeAPowerOfTwo(N1) && (!VT.isVector() || Level <= AfterLegalizeVectorOps)) { SDLoc DL(N); SDValue LogBase2 = BuildLogBase2(N1, DL); EVT ShiftVT = getShiftAmountTy(N0.getValueType()); SDValue Trunc = DAG.getZExtOrTrunc(LogBase2, DL, ShiftVT); return DAG.getNode(ISD::SHL, DL, VT, N0, Trunc); }
Ah nice I missed that, maybe that explains why this patch doesn't test fixed-length vectors, because it's already done. Adding a check for SPLAT_VECTOR seems reasonable to me since it's already got one for BUILD_VECTOR.
Now that the scope of this has increased, I think we should try and add scalable-vector tests for all of the DAGCombines which use isKnownToBeAPowerOfTwo.
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | ||
---|---|---|
3654 | This return false isn't doing the right thing and is skipping the computeKnownBits stuff below. I'm surprised the tests are still passing! Are we missing coverage? I think this should simply be if (C->getAPIntValue().zextOrTrunc(BitWidth).isPowerOf2()) return true, allowing it to fall through to the next checks. |
Perhaps some extra tests for muls by other powers of two?
Are you able to get tests for the (mulhu x, (1 << c)) -> x >> (bitwidth - c) combine? In my experience mulhu can be tricky to source. Maybe D49248 contains some hints.
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | ||
---|---|---|
3650 | nit: constant powers -> a constant power | |
3656 | Clang-format's correct; please remove this extra empty line. |
mulhu cases with constant splat can not be generated from combineShiftToMULH now, I create a new diff D108129 to handle it.
nit: constant powers -> a constant power