[PATCH] [Polly][Refactor][NFC] Simplify and clean the handling of (new) access relations

[Tobias Grosser]

================
Comment at: include/polly/ScopInfo.h:245
@@ +244,3 @@
+  /// @brief Get the new access function imported or set by a pass
+  isl_map *getNewAccessRelation() const;
+
----------------
jdoerfert wrote:
> dpeixott wrote:
> > Does these two methods need to be public? If not it would be good to make them private to avoid confusion over which method to call (e.g. `getAccessRelation`, `getNewAccessRelation`, `getNewestAccessRelation`). I'm not too familiar with the API, so do whatever makes sense.
> > 
> > Otherwise, LGTM.
> I will expose getNewestAccessRelation and replace all uses (only in 2 files outside the ScopInfo itself, at the moment).
> 
> I cannot see any reason not to use the newest access function except in the dependence analysi but the importer runs after that pass and nobody else (at least for now) uses the functionality of a new access relation anyway.
> 
> Thanks!
I agree the three function names are rather confusing. What about just having getOriginalAccessRelation() and getAccessRelation(), with the last implementing what getNewestAccessRelation currently does?

In general all places in Polly should use the newest access relation which becomes the normal access relation. So even the introduction of getNewAccessRelation was probably a bad idea.

================
Comment at: lib/Analysis/ScopInfo.cpp:410
@@ +409,3 @@
+  return isl_pw_multi_aff_from_map(ScheduledAccRel);
+}
+
----------------
I am not sure if pulling this into the MemoryAccess is an improvement. This is a rather complicated sequence of operations that is rather specific to the subsequent use in isl_ast_build_access_from_pw_multi_aff(). As getNewAccessOperand() is rather short and we use this sequence only once, I would suggest to leave the code as it is.

================
Comment at: lib/Analysis/ScopInfo.cpp:633
@@ -620,5 +632,3 @@
   isl_map *S = const_cast<isl_map *>(Schedule);
-  isl_map *AccessRelation = getNewAccessRelation();
-  if (!AccessRelation)
-    AccessRelation = getAccessRelation();
+  isl_map *AccessRelation = getNewestAccessRelation();
   isl_space *Space = isl_space_range(isl_map_get_space(S));
----------------
Having here getAccessRelation() (with the same semantics as now getNewestAccessRelation) seems more understandable. Updates to the access relation should not really affect the code all over polly.

================
Comment at: lib/CodeGen/BlockGenerators.cpp:192
@@ -202,3 +191,3 @@
   Value *NewPointer;
-  if (NewAccRel)
+  if (MA.hasNewAccessRelation())
     NewPointer = getNewAccessOperand(MA);
----------------
Nice.

http://reviews.llvm.org/D5707