[llvm] r253964 - [TableGen] Use std::remove_if instead of manually coded loops that call erase multiple times. NFC
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 24 11:30:27 PST 2015
On Tue, Nov 24, 2015 at 10:06 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> 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?
>
The scope is only there to void having the very similar code for Largest
have to declare differently named temporaries or assume they already
existed and just assign them instead of assigning and declaring. Thoughts?
>
>
>> + 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?
>
I think that makes sense.
>
>
>> + // 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
>>
>
>
--
~Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151124/48e2fd8f/attachment.html>
More information about the llvm-commits
mailing list