[PATCH] D24956: [SelectionDAG] Add expansion and promotion of [US]MUL_LOHI

[Eli Friedman via llvm-commits]

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