[llvm] r237224 - Change a loop in LoopInfo to foreach. NFC

[Pete Cooper]

> On May 13, 2015, at 11:56 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Wed, May 13, 2015 at 11:48 AM, Pete Cooper <peter_cooper at apple.com <mailto:peter_cooper at apple.com>> wrote:
> 
>> On May 13, 2015, at 10:51 AM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>> 
>> 
>> 
>> On Tue, May 12, 2015 at 6:12 PM, Pete Cooper <peter_cooper at apple.com <mailto:peter_cooper at apple.com>> wrote:
>> Author: pete
>> Date: Tue May 12 20:12:09 2015
>> New Revision: 237224
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=237224&view=rev <http://llvm.org/viewvc/llvm-project?rev=237224&view=rev>
>> Log:
>> Change a loop in LoopInfo to foreach.  NFC
>> 
>> Modified:
>>     llvm/trunk/lib/Analysis/LoopInfo.cpp
>> 
>> Modified: llvm/trunk/lib/Analysis/LoopInfo.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/LoopInfo.cpp?rev=237224&r1=237223&r2=237224&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/LoopInfo.cpp?rev=237224&r1=237223&r2=237224&view=diff>
>> ==============================================================================
>> --- llvm/trunk/lib/Analysis/LoopInfo.cpp (original)
>> +++ llvm/trunk/lib/Analysis/LoopInfo.cpp Tue May 12 20:12:09 2015
>> @@ -65,8 +65,8 @@ bool Loop::isLoopInvariant(const Value *
>>  /// hasLoopInvariantOperands - Return true if all the operands of the
>>  /// specified instruction are loop invariant.
>>  bool Loop::hasLoopInvariantOperands(const Instruction *I) const {
>> -  for (unsigned i = 0, e = I->getNumOperands(); i != e; ++i)
>> -    if (!isLoopInvariant(I->getOperand(i)))
>> +  for (auto &Op : I->operands())
>> +    if (!isLoopInvariant(Op))
>>        return false;
>> 
>> Could be written as:
>> 
>>   return std::all_of(I->operands().begin(), I->operands().end(), [&](Value *V) { return isLoopInvariant(V); );
>> 
>> If you like. (but the range-based-ness does make that a bit inconvenient, though std::all_of adds a little readability improvement, I think - at some point we'll just write range-based versions of algorithms like this so we don't have to pass begin/end everywhere)
> Thanks.  Something like this:
> 
> namespace llvm {
> 
> template<typename T, class UnaryPredicate>
>   static inline bool all_of(llvm::iterator_range<T> Range, UnaryPredicate p) {
>   return std::all_of(Range.begin(), Range.end(), p);
> }
> 
> };
> 
> bool Loop::hasLoopInvariantOperands(const Instruction *I) const {
>   return all_of(I->operands(), [&](Value *V) { return isLoopInvariant(V); });
> }
> 
> I chose to constrain is to iterator_range for now just to avoid complicating it to require begin/end of any kind of container. 
> 
> I wouldn't bother with the constraint - I'd just start with a generic ref range R:
Sounds good to me.  Unfortunately operands is an iterator_range which can’t be passed by reference:

../lib/Analysis/LoopInfo.cpp:84:10: error: no matching function for call to 'all_of'
  return all_of(I->operands(), [this](Value *V) { return isLoopInvariant(V); });
         ^~~~~~
../lib/Analysis/LoopInfo.cpp:67:20: note: candidate function [with R = llvm::iterator_range<const llvm::Use *>, UnaryPredicate = (lambda at ../lib/Analysis/LoopInfo.cpp:84:32)] not viable: expects an l-value for 1st argument
static inline bool all_of(R &Range, UnaryPredicate p) {

So I ended up having to define by value and by reference all_of.

template<typename R, class UnaryPredicate>
static inline bool all_of(R Range, UnaryPredicate p) {
  return std::all_of(Range.begin(), Range.end(), p);
}

template<typename R, class UnaryPredicate>
static inline bool all_of(R &Range, UnaryPredicate p) {
  return std::all_of(Range.begin(), Range.end(), p);
}

Pete
> 
> template<typename R, typename UnaryPredicate>
> bool all_of(R &Range, UnaryPredicate p) {
>   return std::all_of(Range.begin(), Range.end(), p);
> }
> 
> And if we one day need to, we can SFINAE this as appropriate. (heh, now I'm imagining writing this as an adaptor... but of course it wouldn't work because we need template argument deduction, etc... but it'd be fun to "rangify(std::all_of, Range, Predicate)" ;))
>  
>  What do you think?
> 
> Cheers,
> Pete
>>  
>> 
>>    return true;
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150513/a37ade0e/attachment.html>