Page MenuHomePhabricator

[DAG] use isConstOrConstSplat in ComputeNumSignBits to optimize SRA
ClosedPublic

Authored by spatel on Oct 17 2016, 10:01 AM.

Details

Summary

This is the minimum patch needed to solve the missed SRA fold that was noted in:
https://reviews.llvm.org/D25485

and partially fixed with:
https://reviews.llvm.org/rL284395

Of course, we could move more of the related analysis functions in DAGCombiner that are currently static/local to have a more unified approach.

Ie, move all of these (some were recently added in D25374)?
isConstantFPBuildVectorOrConstantFP
isConstOrConstSplatFP
isConstantOrConstantVector
isNullConstantOrNullSplatConstant
isOneConstantOrOneSplatConstant
isAllOnesConstantOrAllOnesSplatConstant

It's not clear to me exactly where the handling of undef elements is different and where that is intentional.

Diff Detail

Repository
rL LLVM

Event Timeline

spatel updated this revision to Diff 74865.Oct 17 2016, 10:01 AM
spatel retitled this revision from to [DAG] use isConstOrConstSplat in ComputeNumSignBits to optimize SRA.
spatel updated this object.
spatel added reviewers: mkuper, RKSimon, efriedma.
spatel added a subscriber: llvm-commits.

More consolidation is possible with the ISD namespace helpers, or should the previously listed functions go here too?

namespace ISD {
/// Node predicates

/// If N is a BUILD_VECTOR node whose elements are all the same constant or
/// undefined, return true and return the constant value in \p SplatValue.
bool isConstantSplatVector(const SDNode *N, APInt &SplatValue);

/// Return true if the specified node is a BUILD_VECTOR where all of the
/// elements are ~0 or undef.
bool isBuildVectorAllOnes(const SDNode *N);

/// Return true if the specified node is a BUILD_VECTOR where all of the
/// elements are 0 or undef.
bool isBuildVectorAllZeros(const SDNode *N);

/// Return true if the specified node is a BUILD_VECTOR node of all
/// ConstantSDNode or undef.
bool isBuildVectorOfConstantSDNodes(const SDNode *N);

/// Return true if the specified node is a BUILD_VECTOR node of all
/// ConstantFPSDNode or undef.
bool isBuildVectorOfConstantFPSDNodes(const SDNode *N);

/// Return true if the node has at least one operand and all operands of the
/// specified node are ISD::UNDEF.
bool allOperandsUndef(const SDNode *N);
}  // end llvm:ISD namespace
RKSimon edited edge metadata.Oct 17 2016, 10:19 AM

I agree that all those similar predicates is a bit of pain!

We have issues with handling whether we permit opaque constants and whether UNDEF elements are allowable.

include/llvm/CodeGen/SelectionDAGNodes.h
1419 ↗(On Diff #74865)

Might this be a good time to add an 'AllowUndefs' argument so that we permit cases where there is at least one ConstantSDNode in the splat but other elements can be UNDEF.

RKSimon accepted this revision.Oct 17 2016, 11:29 AM
RKSimon edited edge metadata.

LGTM - I think isConstOrConstSplatFP should probably moved over at the same time as isConstOrConstSplat (separate preliminary commit) followed by the SRA fix - make sense?

We can then investigate adding a "AllowUndefs" args to both of them in a followup commit and start the fun job of consolidating all those predicates.....

This revision is now accepted and ready to land.Oct 17 2016, 11:29 AM
This revision was automatically updated to reflect the committed changes.