[PATCH] D54892: [LAA] Introduce enum for vectorization safety status (NFC).

Ayal Zaks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 18 11:24:55 PST 2018


Ayal added inline comments.


================
Comment at: include/llvm/Analysis/LoopAccessAnalysis.h:100
 
+  /// Type to keep track of the status of the dependence check.
+  enum class VectorizationSafetyStatus {
----------------
> Nice, I didn't know enum classes provide a default implementation for that.

Yeah, they have numerical values, which btw could also be specified.
Would be good to note that the order of elements in the enum is important.


================
Comment at: include/llvm/Analysis/LoopAccessAnalysis.h:221
+    return ShouldRetryWithRuntimeCheck &&
+           Status == VectorizationSafetyStatus::Unsafe;
+  }
----------------
This is redundant, as `ShouldRetryWithRuntimeCheck` implies Unsafe. I.e., can assert !(Should && Safe).
When SafeWithRtChecks is added, in next patch, consider renaming the flag or method (slightly), as they will no longer mean exactly the same thing.


================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:1679
+            if (!RecordDependences &&
+                Status == VectorizationSafetyStatus::Unsafe)
               return false;
----------------
`!isSafeForVectorization()` ?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54892/new/

https://reviews.llvm.org/D54892





More information about the llvm-commits mailing list