[PATCH] D105438: [LV] Re-generate check lines of some fragile tests

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

Thanks, still LGTM with the remaining suggestions.



================
Comment at: llvm/test/Transforms/LoopVectorize/X86/invariant-store-vectorization.ll:55
 ; CHECK-NEXT:    [[TMP12:%.*]] = icmp eq i64 [[INDEX_NEXT]], [[N_VEC]]
-; CHECK-NEXT:    br i1 [[TMP12]], label [[MIDDLE_BLOCK:%.*]], label [[VECTOR_BODY]], [[LOOP5:!llvm.loop !.*]]
 ; CHECK:       middle.block:
----------------
no changes  other than the loop check in this file, so no need to include them?


================
Comment at: llvm/test/Transforms/LoopVectorize/invariant-store-vectorization-2.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -licm -loop-vectorize -force-vector-width=4 -dce -instcombine -licm -S | FileCheck %s
+
----------------
mdchen wrote:
> fhahn wrote:
> > The test should ideally not depend on `licm`. Can you update it so it does not require either LICM run?
> > 
> > Also, it seems this contains similar comments to `llvm/test/Transforms/LoopVectorize/invariant-store-vectorization.ll`. what's the difference?
> This test file used to be part of `llvm/test/Transforms/LoopVectorize/invariant-store-vectorization.ll`. I moved these tests into a separate file because they're auto-generated tests and mix them with hard-coded checks is hard to maintain(mainly because of the numbered metadata).
Alright, if it is just from the existing test that's fine, even though not ideal.


================
Comment at: llvm/test/Transforms/LoopVectorize/invariant-store-vectorization-2.ll:15
+
+; Instcombine'd version of above test. Now the store is no longer of invariant
+; value.
----------------
mdchen wrote:
> fhahn wrote:
> > there's no test above?
> Ahh, I should have looked into the comments carefully. Will rephrase them.
it still says 'of the above test`?


================
Comment at: llvm/test/Transforms/LoopVectorize/invariant-store-vectorization.ll:287
+  %i10 = load i32, i32* %i
+  %i11 = mul nsw i32 %i9, %i10
+  %i12 = srem i32 %i11, 65536
----------------
mdchen wrote:
> fhahn wrote:
> > Why rename those?
> Because `tmp` prefix might conflict with the auto-generated check line variables and `update_test_checks.py` throws warnings for them. I suppose these `tmp` prefixed names are generated by the old version of `opt -instnamer`, and the latest version use `i` instead.
sounds good to me, thanks!


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

https://reviews.llvm.org/D105438