[llvm] r253964 - [TableGen] Use std::remove_if instead of manually coded loops that call erase multiple times. NFC
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 24 10:06:54 PST 2015
On Tue, Nov 24, 2015 at 12:20 AM, Craig Topper via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: ctopper
> Date: Tue Nov 24 02:20:47 2015
> New Revision: 253964
>
> URL: http://llvm.org/viewvc/llvm-project?rev=253964&view=rev
> Log:
> [TableGen] Use std::remove_if instead of manually coded loops that call
> erase multiple times. NFC
>
> Modified:
> llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp
>
> Modified: llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp?rev=253964&r1=253963&r2=253964&view=diff
>
> ==============================================================================
> --- llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp (original)
> +++ llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp Tue Nov 24 02:20:47
> 2015
> @@ -385,55 +385,55 @@ bool EEVT::TypeSet::EnforceSmallerThan(E
> // type size is smaller than the scalar size of the smallest type. For
> // vectors, we also need to make sure that the total size is no larger
> than
> // the size of the smallest type.
> - TypeSet InputSet(Other);
> - MVT Smallest = TypeVec[0];
> - for (unsigned i = 0; i != Other.TypeVec.size(); ++i) {
> - MVT OtherVT = Other.TypeVec[i];
> - // Don't compare vector and non-vector types.
> - if (OtherVT.isVector() != Smallest.isVector())
> - continue;
> - // The getSizeInBits() check here is only needed for vectors, but is
> - // a subset of the scalar check for scalars so no need to qualify.
> - if (OtherVT.getScalarSizeInBits() <= Smallest.getScalarSizeInBits() ||
> - OtherVT.getSizeInBits() < Smallest.getSizeInBits()) {
> - Other.TypeVec.erase(Other.TypeVec.begin()+i--);
> - MadeChange = true;
> + {
>
Is this explicit scope ^ particularly helpful?
> + TypeSet InputSet(Other);
> + MVT Smallest = TypeVec[0];
> + auto I = std::remove_if(Other.TypeVec.begin(), Other.TypeVec.end(),
> + [Smallest](MVT OtherVT) {
>
^ Just as a general idea, I'd like to encourage the use of default capture
by reference ("[&]") for any lambda that's only being used in the scope of
its definition (ie: any standard algorithm or similar device - almost any
time the lambda isn't escaping through a std::function, potentially). I
think it simplifies the code, avoids curious gotchas (I've seen some recent
local/algorithm lambdas with value-by-default ("[=]") which I imagine could
trip someone up on refactoring.
What do you think/what's your preference/model here?
> + // Don't compare vector and non-vector types.
> + if (OtherVT.isVector() != Smallest.isVector())
> + return false;
> + // The getSizeInBits() check here is only needed for vectors, but
> is
> + // a subset of the scalar check for scalars so no need to qualify.
> + return OtherVT.getScalarSizeInBits() <=
> Smallest.getScalarSizeInBits()||
> + OtherVT.getSizeInBits() < Smallest.getSizeInBits();
> + });
> + MadeChange |= I != Other.TypeVec.end(); // If we're about to remove
> types.
> + Other.TypeVec.erase(I, Other.TypeVec.end());
> +
> + if (Other.TypeVec.empty()) {
> + TP.error("Type inference contradiction found, '" +
> InputSet.getName() +
> + "' has nothing larger than '" + getName() +"'!");
> + return false;
> }
> }
>
> - if (Other.TypeVec.empty()) {
> - TP.error("Type inference contradiction found, '" + InputSet.getName()
> +
> - "' has nothing larger than '" + getName() +"'!");
> - return false;
> - }
> -
> // Okay, find the largest type from the other set and remove anything
> the
> // same or smaller from the current set. We need to ensure that the
> scalar
> // type size is larger than the scalar size of the largest type. For
> // vectors, we also need to make sure that the total size is no smaller
> than
> // the size of the largest type.
> - InputSet = TypeSet(*this);
> - MVT Largest = Other.TypeVec[Other.TypeVec.size()-1];
> - for (unsigned i = 0; i != TypeVec.size(); ++i) {
> - MVT OtherVT = TypeVec[i];
> - // Don't compare vector and non-vector types.
> - if (OtherVT.isVector() != Largest.isVector())
> - continue;
> - // The getSizeInBits() check here is only needed for vectors, but is
> - // a subset of the scalar check for scalars so no need to qualify.
> - if (OtherVT.getScalarSizeInBits() >= Largest.getScalarSizeInBits() ||
> - OtherVT.getSizeInBits() > Largest.getSizeInBits()) {
> - TypeVec.erase(TypeVec.begin()+i--);
> - MadeChange = true;
> + {
> + TypeSet InputSet(*this);
> + MVT Largest = Other.TypeVec[Other.TypeVec.size()-1];
> + auto I = std::remove_if(TypeVec.begin(), TypeVec.end(),
> + [Largest](MVT OtherVT) {
> + // Don't compare vector and non-vector types.
> + if (OtherVT.isVector() != Largest.isVector())
> + return false;
> + return OtherVT.getScalarSizeInBits() >=
> Largest.getScalarSizeInBits() ||
> + OtherVT.getSizeInBits() > Largest.getSizeInBits();
> + });
> + MadeChange |= I != TypeVec.end(); // If we're about to remove types.
> + TypeVec.erase(I, TypeVec.end());
> +
> + if (TypeVec.empty()) {
> + TP.error("Type inference contradiction found, '" +
> InputSet.getName() +
> + "' has nothing smaller than '" + Other.getName() +"'!");
> + return false;
> }
> }
>
> - if (TypeVec.empty()) {
> - TP.error("Type inference contradiction found, '" + InputSet.getName()
> +
> - "' has nothing smaller than '" + Other.getName() +"'!");
> - return false;
> - }
> -
> return MadeChange;
> }
>
> @@ -448,17 +448,17 @@ bool EEVT::TypeSet::EnforceVectorEltType
> TypeSet InputSet(*this);
>
> // Filter out all the types which don't have the right element type.
> - for (unsigned i = 0; i != TypeVec.size(); ++i) {
> - assert(isVector(TypeVec[i]) && "EnforceVector didn't work");
> - if (MVT(TypeVec[i]).getVectorElementType().SimpleTy != VT) {
> - TypeVec.erase(TypeVec.begin()+i--);
> - MadeChange = true;
> - }
> - }
> + auto I = std::remove_if(TypeVec.begin(), TypeVec.end(),
> + [VT](MVT VVT) {
> + return VVT.getVectorElementType().SimpleTy != VT;
> + });
> + MadeChange |= I != TypeVec.end();
> + TypeVec.erase(I, TypeVec.end());
>
> if (TypeVec.empty()) { // FIXME: Really want an SMLoc here!
> TP.error("Type inference contradiction found, forcing '" +
> - InputSet.getName() + "' to have a vector element");
> + InputSet.getName() + "' to have a vector element of type " +
> + getEnumName(VT));
> return false;
> }
>
> @@ -535,13 +535,13 @@ bool EEVT::TypeSet::EnforceVectorSubVect
> // Only keep types that have less elements than VTOperand.
> TypeSet InputSet(VTOperand);
>
> - for (unsigned i = 0; i != VTOperand.TypeVec.size(); ++i) {
> - assert(isVector(VTOperand.TypeVec[i]) && "EnforceVector didn't
> work");
> - if (MVT(VTOperand.TypeVec[i]).getVectorNumElements() >= NumElems) {
> - VTOperand.TypeVec.erase(VTOperand.TypeVec.begin()+i--);
> - MadeChange = true;
> - }
> - }
> + auto I = std::remove_if(VTOperand.TypeVec.begin(),
> VTOperand.TypeVec.end(),
> + [NumElems](MVT VVT) {
> + return VVT.getVectorNumElements() >=
> NumElems;
> + });
> + MadeChange |= I != VTOperand.TypeVec.end();
> + VTOperand.TypeVec.erase(I, VTOperand.TypeVec.end());
> +
> if (VTOperand.TypeVec.empty()) { // FIXME: Really want an SMLoc here!
> TP.error("Type inference contradiction found, forcing '" +
> InputSet.getName() + "' to have less vector elements than
> '" +
> @@ -559,13 +559,13 @@ bool EEVT::TypeSet::EnforceVectorSubVect
> // Only keep types that have more elements than 'this'.
> TypeSet InputSet(*this);
>
> - for (unsigned i = 0; i != TypeVec.size(); ++i) {
> - assert(isVector(TypeVec[i]) && "EnforceVector didn't work");
> - if (MVT(TypeVec[i]).getVectorNumElements() <= NumElems) {
> - TypeVec.erase(TypeVec.begin()+i--);
> - MadeChange = true;
> - }
> - }
> + auto I = std::remove_if(TypeVec.begin(), TypeVec.end(),
> + [NumElems](MVT VVT) {
> + return VVT.getVectorNumElements() <=
> NumElems;
> + });
> + MadeChange |= I != TypeVec.end();
> + TypeVec.erase(I, TypeVec.end());
> +
> if (TypeVec.empty()) { // FIXME: Really want an SMLoc here!
> TP.error("Type inference contradiction found, forcing '" +
> InputSet.getName() + "' to have more vector elements than
> '" +
> @@ -597,13 +597,13 @@ bool EEVT::TypeSet::EnforceVectorSameNum
> // Only keep types that have same elements as VTOperand.
> TypeSet InputSet(VTOperand);
>
> - for (unsigned i = 0; i != VTOperand.TypeVec.size(); ++i) {
> - assert(isVector(VTOperand.TypeVec[i]) && "EnforceVector didn't
> work");
> - if (MVT(VTOperand.TypeVec[i]).getVectorNumElements() != NumElems) {
> - VTOperand.TypeVec.erase(VTOperand.TypeVec.begin()+i--);
> - MadeChange = true;
> - }
> - }
> + auto I = std::remove_if(VTOperand.TypeVec.begin(),
> VTOperand.TypeVec.end(),
> + [NumElems](MVT VVT) {
> + return VVT.getVectorNumElements() !=
> NumElems;
> + });
> + MadeChange |= I != VTOperand.TypeVec.end();
> + VTOperand.TypeVec.erase(I, VTOperand.TypeVec.end());
> +
> if (VTOperand.TypeVec.empty()) { // FIXME: Really want an SMLoc here!
> TP.error("Type inference contradiction found, forcing '" +
> InputSet.getName() + "' to have same number elements as '"
> +
> @@ -617,13 +617,13 @@ bool EEVT::TypeSet::EnforceVectorSameNum
> // Only keep types that have same elements as 'this'.
> TypeSet InputSet(*this);
>
> - for (unsigned i = 0; i != TypeVec.size(); ++i) {
> - assert(isVector(TypeVec[i]) && "EnforceVector didn't work");
> - if (MVT(TypeVec[i]).getVectorNumElements() != NumElems) {
> - TypeVec.erase(TypeVec.begin()+i--);
> - MadeChange = true;
> - }
> - }
> + auto I = std::remove_if(TypeVec.begin(), TypeVec.end(),
> + [NumElems](MVT VVT) {
> + return VVT.getVectorNumElements() !=
> NumElems;
> + });
> + MadeChange |= I != TypeVec.end();
> + TypeVec.erase(I, TypeVec.end());
> +
> if (TypeVec.empty()) { // FIXME: Really want an SMLoc here!
> TP.error("Type inference contradiction found, forcing '" +
> InputSet.getName() + "' to have same number elements than
> '" +
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151124/6c081223/attachment-0001.html>
More information about the llvm-commits
mailing list