spatel added inline comments.
================
Comment at: llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-finite.ll:9
+define void @exp_f32(float* nocapture %varray) {
+; CHECK-LABEL: @exp_f32(
+; CHECK-NEXT: entry:
----------------
fpetrogalli wrote:
> I think you are over-testing here. It is enough to check that inside the
> vector body there is a call to the vector function you have listed in the
> mapping. You are not checking for the whole auto-vectorization process here,
> just the vectorization of the function call. Same for all the tests for this
> patch in which you are doing something similar to this one test.
>
> ```
> ; CHECK-LABEL: @exp_f32(
> ; CHECK-LABEL: vector.body:
> ; CHECK: call fast <4 x float> @_ZGVbN4v___expf_finite(<4 x float>
> ```
>
I requested using "utils/update_test_checks.py" to auto-generate the assertions
consistently.
We have standardized on this practice for tests in several passes because it
provides extra test coverage (at the risk of over-testing), and it makes
updating tests in the future nearly automatic.
The time cost of checking the extra lines is negligible vs. the benefit that we
have gotten in finding/avoiding bugs. If the consensus is that it's not worth
it on this particular file, I'm ok with that. But the general trend is
definitely towards auto-generating full checks.
================
Comment at:
llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-finite.ll:69
+ %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
+ %tmp = trunc i64 %indvars.iv to i32
+ %conv = sitofp i32 %tmp to float
----------------
The script should have warned you about using variables named "tmp".
Independent of whether we choose to use the scripted assertions or not, you
should change this value name (even plain "t" for "trunc" is an improvement
over "tmp").
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88154/new/
https://reviews.llvm.org/D88154
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
