[PATCH] D149479: [LV] Enable scalable outer loop vectorization

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2250
 
+static void reportVectorization(OptimizationRemarkEmitter *ORE, Loop *TheLoop,
+                                LoopVectorizationLegality &LVL,
----------------
Could this improvement be split off as a separate patch? It also looks like `LVL` & `LVP` are unused?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp:123
   VPRegionBlock *ParentR = nullptr;
-  if (CurrentLoop) {
+  if (CurrentLoop && CurrentLoop->getLoopDepth() >= TheLoop->getLoopDepth()) {
     auto Iter = Loop2Region.insert({CurrentLoop, nullptr});
----------------
Can this be split off as well with a separate test?


================
Comment at: llvm/test/Transforms/LoopVectorize/RISCV/outer_loop_scalable.ll:98
+
+4:
+  %5 = zext i32 %0 to i64
----------------
Could you update the tests to use names for blocks and values? This would make them much easier to read.


================
Comment at: llvm/test/Transforms/LoopVectorize/X86/outer_loop_no_scalable.ll:1
+; RUN: opt -passes=loop-vectorize -enable-vplan-native-path %s -pass-remarks-analysis=loop-vectorize -disable-output 2>&1 | FileCheck %s
+
----------------
Does this test need to pass an `X86` triple? 


================
Comment at: llvm/test/Transforms/LoopVectorize/outer_loop_test3.ll:3
+; RUN: opt -S -passes=loop-vectorize -enable-vplan-native-path < %s -S -pass-remarks=loop-vectorize 2>%t | FileCheck %s
+; RUN: cat %t | FileCheck %s --check-prefix=CHECK-REMARK
+
----------------
Is this test for the opt remarks? In that case, it would be good to include it in the name (e.g. `outer-loop-remarks.ll`)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149479