[PATCH] D101844: [MicroBenchmarks] Add initial loop vectorization benchmarks.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D101844#2736630 <https://reviews.llvm.org/D101844#2736630>, @Meinersbur wrote:

> Do you have an example output? How long does it take to execute the benchmark?
>
> The validation uses a floating-point comparison using `==`, could you add a comment that this is intended? I am a bit worried that if I compile the test-suite with `CMAKE_CXX_FLAGS=-ffast-math` (mostly to measure performance), this will fail.
>
> The comparison is part of the time measurements. This doesn't seem useful to be for benchmarking the vectorized and non-vectorized versions together. If the only things we are interested in is the correctness, we don't need Google Benchmark. Use Google test instead?





================
Comment at: MicroBenchmarks/LoopVectorization/MathFunctions.cpp:54-55
+      // Treat nans/infs as matching, if both versions produce nan/inf.
+      if (C[i] != CNovec[i] && !(isnan(C[i]) && isnan(CNovec[i])) &&
+          !(isinf(C[i]) && isinf(CNovec[i]))) {
+        std::cerr << "ERROR: autovec result different to scalar result " << C[i]
----------------
lebedev.ri wrote:
> I'm not confident this is
> 1. consistent with the rules
> 2. will withstand the whatever compiler optimization levels test-suite is compiled with
> 
> Perhaps you want to be on the safe side and do `fpclassify()`,
> fail if they mismatch, and compare value-equality if they are normal/subnormal?
Thanks, I updated the patch to fall back to `fpclassify` if there's a mis-match. 

> consistent with the rules

Do you mean rules for the test-suite?

> will withstand the whatever compiler optimization levels test-suite is compiled with

I think at the moment, the only issue at the moment could be that the compiler picks a different vector math function with `-ffast-math` or the user specifies a vector library that does not guarantee the same results as the scalar version.

The `-fast-math` case should be handled in the latest version, but the user specified vector library is not. Not sure what we can do about that case and how/if other tests deal with that issue.



Repository:
  rT test-suite

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

https://reviews.llvm.org/D101844