[PATCH] D143422: [LV] Update logic for calculating register usage due to invariants

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

Thanks for making the changes. I've just left a few more comments on the test.



================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/reg-usage.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+;RUN: opt -mtriple arm64-linux -passes=loop-vectorize -mattr=+sve -debug-only=loop-vectorize -disable-output <%s 2>&1 | FileCheck %s
+
----------------
nit: `; Run:`


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/reg-usage.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+;RUN: opt -mtriple arm64-linux -passes=loop-vectorize -mattr=+sve -debug-only=loop-vectorize -disable-output <%s 2>&1 | FileCheck %s
+
----------------
sdesmalen wrote:
> nit: `; Run:`
If the test uses -debug-only, then it also needs `REQUIRES: asserts`


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/reg-usage.ll:4
+
+; Check that this doesn't crash while finding register usage
+
----------------
Can you add a more elaborate description here of what it's testing?



================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/reg-usage.ll:7
+ at string = internal unnamed_addr constant [5 x i8] c"abcd\00", align 1
+define dso_local i32 @get_invariant_reg_usage(ptr %z) local_unnamed_addr {
+;CHECK: LV: Checking a loop in 'get_invariant_reg_usage'
----------------
nit: Can you remove the dso_local and the local_unnamed_addr, as they're not needed.


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/reg-usage.ll:28
+  %a = phi ptr [ %5, %loopbody ], [ %4, %loopbody.preheader ]
+  %b = phi ptr [ %6, %loopbody ], [ @string, %loopbody.preheader ]
+  %len_input = phi i128 [ %len, %loopbody ], [ %2, %loopbody.preheader ]
----------------
Can this test be simplified a bit further? A lot of the instructions here (e.g. some the PHIs in the loop, the load %0 and subsequent GEP, etc) are not necessary for this test.


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

https://reviews.llvm.org/D143422