<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 24, 2015 at 11:30 AM, Craig Topper <span dir="ltr"><<a href="mailto:craig.topper@gmail.com" target="_blank">craig.topper@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Tue, Nov 24, 2015 at 10:06 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 24, 2015 at 12:20 AM, Craig Topper via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: ctopper<br>
Date: Tue Nov 24 02:20:47 2015<br>
New Revision: 253964<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=253964&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=253964&view=rev</a><br>
Log:<br>
[TableGen] Use std::remove_if instead of manually coded loops that call erase multiple times. NFC<br>
<br>
Modified:<br>
llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp<br>
<br>
Modified: llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp?rev=253964&r1=253963&r2=253964&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp?rev=253964&r1=253963&r2=253964&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp (original)<br>
+++ llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp Tue Nov 24 02:20:47 2015<br>
@@ -385,55 +385,55 @@ bool EEVT::TypeSet::EnforceSmallerThan(E<br>
// type size is smaller than the scalar size of the smallest type. For<br>
// vectors, we also need to make sure that the total size is no larger than<br>
// the size of the smallest type.<br>
- TypeSet InputSet(Other);<br>
- MVT Smallest = TypeVec[0];<br>
- for (unsigned i = 0; i != Other.TypeVec.size(); ++i) {<br>
- MVT OtherVT = Other.TypeVec[i];<br>
- // Don't compare vector and non-vector types.<br>
- if (OtherVT.isVector() != Smallest.isVector())<br>
- continue;<br>
- // The getSizeInBits() check here is only needed for vectors, but is<br>
- // a subset of the scalar check for scalars so no need to qualify.<br>
- if (OtherVT.getScalarSizeInBits() <= Smallest.getScalarSizeInBits() ||<br>
- OtherVT.getSizeInBits() < Smallest.getSizeInBits()) {<br>
- Other.TypeVec.erase(Other.TypeVec.begin()+i--);<br>
- MadeChange = true;<br>
+ {<br></blockquote><div><br>Is this explicit scope ^ particularly helpful?<br></div></div></div></div></blockquote><div><br></div></div></div><div>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?</div></div></div></div></blockquote><div><br></div><div>Ah, I see, thanks for the explanation (I skimmed the patch & was wondering why I was seeing the same diff twice, but figured I might've just scrolled weirdly or something... ;)). <br><br>That makes sense - and it looks like the differences are enoguh that it'd be difficult/annoying/not beneficial to refactor out into some generic routine that could be used easily for both largest and smallest. And yeah, dropping the scope & having the first one have declarations and the second not - that's probably what I'd do, but the explicit scopes aren't a bad tool to use there either, no worries.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ TypeSet InputSet(Other);<br>
+ MVT Smallest = TypeVec[0];<br>
+ auto I = std::remove_if(Other.TypeVec.begin(), Other.TypeVec.end(),<br>
+ [Smallest](MVT OtherVT) {<br></blockquote><div><br></div><div>^ 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. <br><br>What do you think/what's your preference/model here?</div></div></div></div></blockquote><div><br></div></span><div>I think that makes sense.</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ // Don't compare vector and non-vector types.<br>
+ if (OtherVT.isVector() != Smallest.isVector())<br>
+ return false;<br>
+ // The getSizeInBits() check here is only needed for vectors, but is<br>
+ // a subset of the scalar check for scalars so no need to qualify.<br>
+ return OtherVT.getScalarSizeInBits() <= Smallest.getScalarSizeInBits()||<br>
+ OtherVT.getSizeInBits() < Smallest.getSizeInBits();<br>
+ });<br>
+ MadeChange |= I != Other.TypeVec.end(); // If we're about to remove types.<br>
+ Other.TypeVec.erase(I, Other.TypeVec.end());<br>
+<br>
+ if (Other.TypeVec.empty()) {<br>
+ TP.error("Type inference contradiction found, '" + InputSet.getName() +<br>
+ "' has nothing larger than '" + getName() +"'!");<br>
+ return false;<br>
}<br>
}<br>
<br>
- if (Other.TypeVec.empty()) {<br>
- TP.error("Type inference contradiction found, '" + InputSet.getName() +<br>
- "' has nothing larger than '" + getName() +"'!");<br>
- return false;<br>
- }<br>
-<br>
// Okay, find the largest type from the other set and remove anything the<br>
// same or smaller from the current set. We need to ensure that the scalar<br>
// type size is larger than the scalar size of the largest type. For<br>
// vectors, we also need to make sure that the total size is no smaller than<br>
// the size of the largest type.<br>
- InputSet = TypeSet(*this);<br>
- MVT Largest = Other.TypeVec[Other.TypeVec.size()-1];<br>
- for (unsigned i = 0; i != TypeVec.size(); ++i) {<br>
- MVT OtherVT = TypeVec[i];<br>
- // Don't compare vector and non-vector types.<br>
- if (OtherVT.isVector() != Largest.isVector())<br>
- continue;<br>
- // The getSizeInBits() check here is only needed for vectors, but is<br>
- // a subset of the scalar check for scalars so no need to qualify.<br>
- if (OtherVT.getScalarSizeInBits() >= Largest.getScalarSizeInBits() ||<br>
- OtherVT.getSizeInBits() > Largest.getSizeInBits()) {<br>
- TypeVec.erase(TypeVec.begin()+i--);<br>
- MadeChange = true;<br>
+ {<br>
+ TypeSet InputSet(*this);<br>
+ MVT Largest = Other.TypeVec[Other.TypeVec.size()-1];<br>
+ auto I = std::remove_if(TypeVec.begin(), TypeVec.end(),<br>
+ [Largest](MVT OtherVT) {<br>
+ // Don't compare vector and non-vector types.<br>
+ if (OtherVT.isVector() != Largest.isVector())<br>
+ return false;<br>
+ return OtherVT.getScalarSizeInBits() >= Largest.getScalarSizeInBits() ||<br>
+ OtherVT.getSizeInBits() > Largest.getSizeInBits();<br>
+ });<br>
+ MadeChange |= I != TypeVec.end(); // If we're about to remove types.<br>
+ TypeVec.erase(I, TypeVec.end());<br>
+<br>
+ if (TypeVec.empty()) {<br>
+ TP.error("Type inference contradiction found, '" + InputSet.getName() +<br>
+ "' has nothing smaller than '" + Other.getName() +"'!");<br>
+ return false;<br>
}<br>
}<br>
<br>
- if (TypeVec.empty()) {<br>
- TP.error("Type inference contradiction found, '" + InputSet.getName() +<br>
- "' has nothing smaller than '" + Other.getName() +"'!");<br>
- return false;<br>
- }<br>
-<br>
return MadeChange;<br>
}<br>
<br>
@@ -448,17 +448,17 @@ bool EEVT::TypeSet::EnforceVectorEltType<br>
TypeSet InputSet(*this);<br>
<br>
// Filter out all the types which don't have the right element type.<br>
- for (unsigned i = 0; i != TypeVec.size(); ++i) {<br>
- assert(isVector(TypeVec[i]) && "EnforceVector didn't work");<br>
- if (MVT(TypeVec[i]).getVectorElementType().SimpleTy != VT) {<br>
- TypeVec.erase(TypeVec.begin()+i--);<br>
- MadeChange = true;<br>
- }<br>
- }<br>
+ auto I = std::remove_if(TypeVec.begin(), TypeVec.end(),<br>
+ [VT](MVT VVT) {<br>
+ return VVT.getVectorElementType().SimpleTy != VT;<br>
+ });<br>
+ MadeChange |= I != TypeVec.end();<br>
+ TypeVec.erase(I, TypeVec.end());<br>
<br>
if (TypeVec.empty()) { // FIXME: Really want an SMLoc here!<br>
TP.error("Type inference contradiction found, forcing '" +<br>
- InputSet.getName() + "' to have a vector element");<br>
+ InputSet.getName() + "' to have a vector element of type " +<br>
+ getEnumName(VT));<br>
return false;<br>
}<br>
<br>
@@ -535,13 +535,13 @@ bool EEVT::TypeSet::EnforceVectorSubVect<br>
// Only keep types that have less elements than VTOperand.<br>
TypeSet InputSet(VTOperand);<br>
<br>
- for (unsigned i = 0; i != VTOperand.TypeVec.size(); ++i) {<br>
- assert(isVector(VTOperand.TypeVec[i]) && "EnforceVector didn't work");<br>
- if (MVT(VTOperand.TypeVec[i]).getVectorNumElements() >= NumElems) {<br>
- VTOperand.TypeVec.erase(VTOperand.TypeVec.begin()+i--);<br>
- MadeChange = true;<br>
- }<br>
- }<br>
+ auto I = std::remove_if(VTOperand.TypeVec.begin(), VTOperand.TypeVec.end(),<br>
+ [NumElems](MVT VVT) {<br>
+ return VVT.getVectorNumElements() >= NumElems;<br>
+ });<br>
+ MadeChange |= I != VTOperand.TypeVec.end();<br>
+ VTOperand.TypeVec.erase(I, VTOperand.TypeVec.end());<br>
+<br>
if (VTOperand.TypeVec.empty()) { // FIXME: Really want an SMLoc here!<br>
TP.error("Type inference contradiction found, forcing '" +<br>
InputSet.getName() + "' to have less vector elements than '" +<br>
@@ -559,13 +559,13 @@ bool EEVT::TypeSet::EnforceVectorSubVect<br>
// Only keep types that have more elements than 'this'.<br>
TypeSet InputSet(*this);<br>
<br>
- for (unsigned i = 0; i != TypeVec.size(); ++i) {<br>
- assert(isVector(TypeVec[i]) && "EnforceVector didn't work");<br>
- if (MVT(TypeVec[i]).getVectorNumElements() <= NumElems) {<br>
- TypeVec.erase(TypeVec.begin()+i--);<br>
- MadeChange = true;<br>
- }<br>
- }<br>
+ auto I = std::remove_if(TypeVec.begin(), TypeVec.end(),<br>
+ [NumElems](MVT VVT) {<br>
+ return VVT.getVectorNumElements() <= NumElems;<br>
+ });<br>
+ MadeChange |= I != TypeVec.end();<br>
+ TypeVec.erase(I, TypeVec.end());<br>
+<br>
if (TypeVec.empty()) { // FIXME: Really want an SMLoc here!<br>
TP.error("Type inference contradiction found, forcing '" +<br>
InputSet.getName() + "' to have more vector elements than '" +<br>
@@ -597,13 +597,13 @@ bool EEVT::TypeSet::EnforceVectorSameNum<br>
// Only keep types that have same elements as VTOperand.<br>
TypeSet InputSet(VTOperand);<br>
<br>
- for (unsigned i = 0; i != VTOperand.TypeVec.size(); ++i) {<br>
- assert(isVector(VTOperand.TypeVec[i]) && "EnforceVector didn't work");<br>
- if (MVT(VTOperand.TypeVec[i]).getVectorNumElements() != NumElems) {<br>
- VTOperand.TypeVec.erase(VTOperand.TypeVec.begin()+i--);<br>
- MadeChange = true;<br>
- }<br>
- }<br>
+ auto I = std::remove_if(VTOperand.TypeVec.begin(), VTOperand.TypeVec.end(),<br>
+ [NumElems](MVT VVT) {<br>
+ return VVT.getVectorNumElements() != NumElems;<br>
+ });<br>
+ MadeChange |= I != VTOperand.TypeVec.end();<br>
+ VTOperand.TypeVec.erase(I, VTOperand.TypeVec.end());<br>
+<br>
if (VTOperand.TypeVec.empty()) { // FIXME: Really want an SMLoc here!<br>
TP.error("Type inference contradiction found, forcing '" +<br>
InputSet.getName() + "' to have same number elements as '" +<br>
@@ -617,13 +617,13 @@ bool EEVT::TypeSet::EnforceVectorSameNum<br>
// Only keep types that have same elements as 'this'.<br>
TypeSet InputSet(*this);<br>
<br>
- for (unsigned i = 0; i != TypeVec.size(); ++i) {<br>
- assert(isVector(TypeVec[i]) && "EnforceVector didn't work");<br>
- if (MVT(TypeVec[i]).getVectorNumElements() != NumElems) {<br>
- TypeVec.erase(TypeVec.begin()+i--);<br>
- MadeChange = true;<br>
- }<br>
- }<br>
+ auto I = std::remove_if(TypeVec.begin(), TypeVec.end(),<br>
+ [NumElems](MVT VVT) {<br>
+ return VVT.getVectorNumElements() != NumElems;<br>
+ });<br>
+ MadeChange |= I != TypeVec.end();<br>
+ TypeVec.erase(I, TypeVec.end());<br>
+<br>
if (TypeVec.empty()) { // FIXME: Really want an SMLoc here!<br>
TP.error("Type inference contradiction found, forcing '" +<br>
InputSet.getName() + "' to have same number elements than '" +<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>
</blockquote></div></div></div><span class="HOEnZb"><font color="#888888"><br><br clear="all"><div><br></div>-- <br><div>~Craig</div>
</font></span></div></div>
</blockquote></div><br></div></div>