[PATCH] D24956: [SelectionDAG] Add expansion and promotion of [US]MUL_LOHI
Eli Friedman via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 4 09:52:05 PDT 2016
efriedma added inline comments.
> TargetLowering.h:147
> + enum class MulExpansion {
> + None, // Do not actually expand anything
> + Split, // Split into MUL + MULH[US]
It doesn't look like None is actually used anywhere... did you mean to use it somehow?
> TargetLowering.cpp:3156
> + SumHi = DAG.getNode(ISD::ADDE, dl, CarryVTs, SumHi, Hi, SumLo.getValue(1));
> + SumLo = DAG.getNode(ISD::ADDC, dl, CarryVTs, SumLo, Lo);
> + Result.push_back(SumLo);
Why are you using ADDC+ADDE rather than just plain ADD? It seems strange to split the operation, given that it might be legal. (I might be missing something here, though.)
> SparcISelLowering.cpp:1696
> if (Subtarget->is64Bit()) {
> + // FIXME: Sparc provides these multiplies, but we don't have them yet.
> + setOperationAction(ISD::UMUL_LOHI, MVT::i32, Promote);
This FIXME doesn't make sense here; Promote is just the right thing to do here.
> X86ISelLowering.cpp:32532
> + if (VT.getScalarSizeInBits() >= 64)
> + return MulExpansion::Scalarize;
> + return MulExpansion::Split;
I'm confused; you're returning "Scalarize" for scalar types?
https://reviews.llvm.org/D24956
More information about the llvm-commits
mailing list