[PATCH] D45872: [DA] Enable -da-delinearize by default

Philip Pfaffe via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 4 02:11:27 PDT 2018


philip.pfaffe added a comment.

Some inline nits. What worries me slightly is that the test oracle changed in `GCD.ll`. Why is that? Where the tests plainly wrong before?



================
Comment at: lib/Analysis/DependenceAnalysis.cpp:3260
+  for (int i = 0; i < size; ++i) {
+    if (i != 0) {
+      auto isKnownLessThan = [=](const SCEV *S, const SCEV *Size) {
----------------
I'd prefer a `continue` over a structured block here.


================
Comment at: lib/Analysis/DependenceAnalysis.cpp:3261
+    if (i != 0) {
+      auto isKnownLessThan = [=](const SCEV *S, const SCEV *Size) {
+        // First unify to the same type
----------------
`tryDeliniarize` is a 300LOC function. Instead of the lambda, why not make this a static function?


================
Comment at: lib/Analysis/DependenceAnalysis.cpp:3286
+
+        // Check using using normal isKnownNegative
+        const SCEV *LimitedBound = SE->getMinusSCEV(
----------------
Double 'using'.


https://reviews.llvm.org/D45872





More information about the llvm-commits mailing list