llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.08k stars 11.59k forks source link

[DAG] SelectionDAGBuilder - add ISD::TRUNCATE NSW/NUW flags support and initial testing #89237

Open RKSimon opened 5 months ago

RKSimon commented 5 months ago

This should do it:

void SelectionDAGBuilder::visitTrunc(const User &I) {
  // TruncInst cannot be a no-op cast because sizeof(src) > sizeof(dest).
  SDValue N = getValue(I.getOperand(0));
  EVT DestVT = DAG.getTargetLoweringInfo().getValueType(DAG.getDataLayout(),
                                                        I.getType());

  SDNodeFlags Flags;
  if (auto *TI = dyn_cast<TruncInst>(&I)) {
    Flags.setNoSignedWrap(TI->hasNoSignedWrap());
    Flags.setNoUnsignedWrap(TI->hasNoUnsignedWrap());
  }

  setValue(&I, DAG.getNode(ISD::TRUNCATE, getCurSDLoc(), DestVT, N, Flags));
}

But then we need to add a lot of general test coverage: https://alive2.llvm.org/ce/z/-wXZPE (a couple of trivial examples)

CC @elhewaty @nikic @goldsteinn

elhewaty commented 5 months ago

@RKSimon, I will work on this first.

asb commented 5 months ago

I was looking in this general direction because I was seeking to handle more cases in the IR TypePromotion pass and had issues with trunc in my desired transformation lowering to additional instructions meaning I didn't get the win I was hoping for. Generating a trunc nuw would work in my case, but as noted here that information is lost upon conversion to SDag.

I was looking to produce ISD::AssertZext+ISD::TRUNCATE for trunc nuw which of course benefits from the existing codepaths. Did you consider that approach?

RKSimon commented 5 months ago

I'd assumed we'd want to try and keep the IR flags alive in the DAG, but ISD::AssertZext/AssertSext do have the benefit that they'd work with the existing DAG valuetracking, so it would probably mean that the X86 PACKSS/PACKUS node matching would just work with existing computeKnownBits/ComputeNumSignBits calls.

Something I did hit not so long ago is ISD::AssertZext/AssertSext often interfere with poison/freeze handling - as we can get stuck with freeze(assertzext(x)).

elhewaty commented 4 months ago

@RKSimon Sorry for my late response. I am working on this now, but I have some problems with testing, I know that SelectionDAG cannot recognize nsw/nuw flags but what is the output of the SDAG after adding this feature, I mean how can I differentiate between the correct output and the other? how can I know that DAG has successfully supported this feature?