[polly] r262033 - ScopDetect/Info: Add option to disable invariant load hoisting

[Tobias Grosser via llvm-commits]

On 02/27/2016 11:33 AM, Johannes Doerfert wrote:
> On 02/26, Tobias Grosser via llvm-commits wrote:
>> Author: grosser
>> Date: Fri Feb 26 10:43:35 2016
>> New Revision: 262033
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=262033&view=rev
>> Log:
>> ScopDetect/Info: Add option to disable invariant load hoisting
>>
>> This is helpful for test case reduction and other experiments.
>>
>> Modified:
>>      polly/trunk/include/polly/ScopDetection.h
>>      polly/trunk/lib/Analysis/ScopDetection.cpp
>>      polly/trunk/lib/Analysis/ScopInfo.cpp
>>
>> Modified: polly/trunk/include/polly/ScopDetection.h
>> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/include/polly/ScopDetection.h?rev=262033&r1=262032&r2=262033&view=diff
>> ==============================================================================
>> --- polly/trunk/include/polly/ScopDetection.h (original)
>> +++ polly/trunk/include/polly/ScopDetection.h Fri Feb 26 10:43:35 2016
>> @@ -110,6 +110,7 @@ extern bool PollyTrackFailures;
>>   extern bool PollyDelinearize;
>>   extern bool PollyUseRuntimeAliasChecks;
>>   extern bool PollyProcessUnprofitable;
>> +extern bool PollyInvariantLoadHoisting;
>>
>>   /// @brief A function attribute which will cause Polly to skip the function
>>   extern llvm::StringRef PollySkipFnAttr;
>>
>> Modified: polly/trunk/lib/Analysis/ScopDetection.cpp
>> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopDetection.cpp?rev=262033&r1=262032&r2=262033&view=diff
>> ==============================================================================
>> --- polly/trunk/lib/Analysis/ScopDetection.cpp (original)
>> +++ polly/trunk/lib/Analysis/ScopDetection.cpp Fri Feb 26 10:43:35 2016
>> @@ -171,6 +171,12 @@ static cl::opt<bool>
>>                   cl::Hidden, cl::init(false), cl::ZeroOrMore,
>>                   cl::cat(PollyCategory));
>>
>> +bool polly::PollyInvariantLoadHoisting;
>> +static cl::opt<bool, true> XPollyInvariantLoadHoisting(
>> +    "polly-invariant-load-hoisting", cl::desc("Hoist invariant loads."),
>> +    cl::location(PollyInvariantLoadHoisting), cl::Hidden, cl::ZeroOrMore,
>> +    cl::init(true), cl::cat(PollyCategory));
>> +
>>   /// @brief The minimal trip count under which loops are considered unprofitable.
>>   static const unsigned MIN_LOOP_TRIP_COUNT = 8;
>>
>> @@ -304,6 +310,9 @@ bool ScopDetection::onlyValidRequiredInv
>>       InvariantLoadsSetTy &RequiredILS, DetectionContext &Context) const {
>>     Region &CurRegion = Context.CurRegion;
>>
>> +  if (!PollyInvariantLoadHoisting && !RequiredILS.empty())
>> +    return false;
>> +
>>     for (LoadInst *Load : RequiredILS)
>>       if (!isHoistableLoad(Load, CurRegion, *LI, *SE))
>>         return false;
>>
>> Modified: polly/trunk/lib/Analysis/ScopInfo.cpp
>> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopInfo.cpp?rev=262033&r1=262032&r2=262033&view=diff
>> ==============================================================================
>> --- polly/trunk/lib/Analysis/ScopInfo.cpp (original)
>> +++ polly/trunk/lib/Analysis/ScopInfo.cpp Fri Feb 26 10:43:35 2016
>> @@ -3116,27 +3116,28 @@ void Scop::verifyInvariantLoads(ScopDete
>>   }
>>
>>   void Scop::hoistInvariantLoads(ScopDetection &SD) {
>> -  isl_union_map *Writes = getWrites();
>> -  for (ScopStmt &Stmt : *this) {
>> -
>> -    MemoryAccessList InvariantAccesses;
>> -
>> -    for (MemoryAccess *Access : Stmt)
>> -      if (isHoistableAccess(Access, Writes))
>> -        InvariantAccesses.push_front(Access);
>> -
>> -    // We inserted invariant accesses always in the front but need them to be
>> -    // sorted in a "natural order". The statements are already sorted in reverse
>> -    // post order and that suffices for the accesses too. The reason we require
>> -    // an order in the first place is the dependences between invariant loads
>> -    // that can be caused by indirect loads.
>> -    InvariantAccesses.reverse();
>> -
>> -    // Transfer the memory access from the statement to the SCoP.
>> -    Stmt.removeMemoryAccesses(InvariantAccesses);
>> -    addInvariantLoads(Stmt, InvariantAccesses);
>> +  if (PollyInvariantLoadHoisting) {
>> +    isl_union_map *Writes = getWrites();
>> +    for (ScopStmt &Stmt : *this) {
>> +      MemoryAccessList InvariantAccesses;
>> +
>> +      for (MemoryAccess *Access : Stmt)
>> +        if (isHoistableAccess(Access, Writes))
>> +          InvariantAccesses.push_front(Access);
>> +
>> +      // We inserted invariant accesses always in the front but need them to be
>> +      // sorted in a "natural order". The statements are already sorted in
>> +      // reverse post order and that suffices for the accesses too. The reason
>> +      // we require an order in the first place is the dependences between
>> +      // invariant loads that can be caused by indirect loads.
>> +      InvariantAccesses.reverse();
>> +
>> +      // Transfer the memory access from the statement to the SCoP.
>> +      Stmt.removeMemoryAccesses(InvariantAccesses);
>> +      addInvariantLoads(Stmt, InvariantAccesses);
>> +    }
>> +    isl_union_map_free(Writes);
>>     }
>> -  isl_union_map_free(Writes);
>>
>>     verifyInvariantLoads(SD);
>>   }
>
> Just for the sake of completeness:
>
> if (!PollyInvariantLoadHoisting) {
>    verifyInvariantLoads(SD);
>    return;
> }
>
> Would have allowed to keep the old (smaller) indention.

I am confused. How is this identical?

I don't want to skip verification, I just want to avoid hoisting loads. 
It seems your patch does still hoist loads, but then does not verify if 
all loads have been hoisted.

Best,
Tobias