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

[Johannes Doerfert via llvm-commits]

On 02/27, Tobias Grosser wrote:
> 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.
No it does not. Revert your patch and add these 4 lines at the beginning
of the method. It will verify and leave the method if
PollyInvariantLoadHoisting is false and otherwise behave as before
(since we did not change anything later).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 213 bytes
Desc: Digital signature
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160227/9d64f183/attachment.sig>