[PATCH] D15307: ScopInfo: Harmonize the different ARRAY_KINDs

[Michael Kruse via llvm-commits]

Meinersbur added a comment.

I welcome any consistent naming scheme.

MemoryAccess::print still prints "[SCALAR: 0]" or "[SCALAR: 1]". It should maybe print the kind instead, although this requires a lot of unit tests to change.

KIND_VALUE fits because it originates from an llvm::Value, analogously to a KIND_PHI originating from an llvm:PHINode. The reason I did not name like this in my first commit is that we have WRITE MemoryAccesses to such "values", but values are not written to (like variables), but defined.

I kind of liked the distinction of implicit/explicit (i.e. whether it comes from an explicit LoadInst/StoreInst or it is a result of how we model scalars), but it might also be orthogonal to the array kind.


================
Comment at: include/polly/ScopInfo.h:120
@@ +119,3 @@
+  enum KIND {
+    /// * KIND_ARRAY: Models a multi-dimensional array
+    ///
----------------
array of arbitrary dimensionality.

multi-dimensional could be understood as at least 2 dimensions.

================
Comment at: include/polly/ScopInfo.h:605
@@ -581,3 +604,3 @@
                ArrayRef<const SCEV *> Subscripts, ArrayRef<const SCEV *> Sizes,
-               Value *AccessValue, AccessOrigin Origin, StringRef BaseName);
+               Value *AccessValue, ARRAYKIND ArrayKind, StringRef BaseName);
   ~MemoryAccess();
----------------
ScopArrayInfo::KIND, ARRAYKIND, KIND_ARRAY is confusing.

I don't think ScopArrayInfo::KIND is too long to require a typedef, but if, it should have the same name as the declaration in ScopArrayInfo. The individual values (e.g. ScopArrayInfo::KIND_ARRAY) still need to be fully qualified.

 [[ http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly | LLVM's style guide ]] says the enum type names should not be all caps, and uses "Kind" as suffix instead of prefix.

================
Comment at: include/polly/ScopInfo.h:700
@@ -676,3 +699,3 @@
   /// @brief Whether this is an access of an explicit load or store in the IR.
-  bool isExplicit() const { return Origin == EXPLICIT; }
+  bool isKindArray() const { return ArrayKind == ScopArrayInfo::KIND_ARRAY; }
 
----------------
The function name sounds gramatically wrong.


http://reviews.llvm.org/D15307