This is an archive of the discontinued LLVM Phabricator instance.

[SelectionDAG][AArch64] Constant fold in SelectionDAG::getVScale if VScaleMin==VScaleMax.
ClosedPublic

Authored by craig.topper on Mar 1 2023, 1:03 PM.

Diff Detail

Event Timeline

craig.topper created this revision.Mar 1 2023, 1:03 PM
craig.topper requested review of this revision.Mar 1 2023, 1:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2023, 1:03 PM
sdesmalen added inline comments.Mar 2 2023, 12:31 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
14846 ↗(On Diff #501649)

Please use clang-format

14850–14857 ↗(On Diff #501649)

Seems easier to write:

unsigned VScaleMin = Attr.getVScaleRangeMin();
if (std::optional<unsigned> VScaleMax = Attr.getVScaleRangeMax())
  if (*VScaleMax == VScaleMin)
    return DAG.getConstant(VScaleMin * N->getConstantOperandAPInt(0), SDLoc(N),
                           N->getValueType(0));
return SDValue();

?

Did you also try to see if there is additional value doing this transform at the LLVM IR layer? (I don't know if that could unlock other optimisations since ConstantFolding kicks in earlier)

Did you also try to see if there is additional value doing this transform at the LLVM IR layer? (I don't know if that could unlock other optimisations since ConstantFolding kicks in earlier)

I think it’s already in InstSimplify.

Did you also try to see if there is additional value doing this transform at the LLVM IR layer? (I don't know if that could unlock other optimisations since ConstantFolding kicks in earlier)

I think it’s already in InstSimplify.

Good, thanks for confirming!

Not a strongly held view but is there value in restricting the combine to pre-operation legalisation? I ask because I wonder if they'll be a point during legalisation where somebody specifically wants the ISD::VSCALE node. Perhaps a different/better option is to move the logic into SelectionDAG::getVScale(), which can take a default false bool to disable the optimisation?

Move to getVScale

craig.topper retitled this revision from [DAGCombiner][AArch64] Constant fold ISD::VSCALE if VScaleMin==VScaleMax. to [SelectionDAG][AArch64] Constant fold in SelectionDAG::getVScale if VScaleMin==VScaleMax..Mar 2 2023, 9:23 AM
paulwalker-arm accepted this revision.Mar 2 2023, 10:34 AM
This revision is now accepted and ready to land.Mar 2 2023, 10:34 AM
This revision was landed with ongoing or failed builds.Mar 2 2023, 12:08 PM
This revision was automatically updated to reflect the committed changes.