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

Mindong Chen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jul 10 08:32:04 PDT 2021


mdchen added inline comments.


================
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
+
----------------
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).


================
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.
----------------
fhahn wrote:
> there's no test above?
Ahh, I should have looked into the comments carefully. Will rephrase them.


================
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
----------------
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.


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

https://reviews.llvm.org/D105438



More information about the llvm-commits mailing list