[PATCH] D119121: [test-suite] Add unit tests for vectorizer memory runtime checks.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: SingleSource/UnitTests/Vectorizer/runtime-checks.cpp:124
+
+  for (int i = -200; i <= 200; i++)
+    CheckWithOffsetSecond(i);
----------------
Meinersbur wrote:
> 
Adjusted, thanks!


================
Comment at: SingleSource/UnitTests/Vectorizer/runtime-checks.cpp:208
+  {
+    DEFINE_SCALAR_AND_VECTOR_FN2(_Pragma(
+        "clang loop unroll(disable)") for (unsigned i = 0; i < TC;
----------------
dmgreen wrote:
> Should this one have this loop unroll Pragma here? It's different to the others.
I *think* Clang performs runtime unrolling here before vectorization and that's why I added the pragma. It's not strictly necessary though.


================
Comment at: SingleSource/UnitTests/Vectorizer/runtime-checks.cpp:71
+    // Run scalar function to generate reference output.
+    auto *ReferenceStart = &Reference[0] + NumArrayElements / 2;
+    ScalarFn(ReferenceStart + Offset, ReferenceStart, N);
----------------
Meinersbur wrote:
> fhahn wrote:
> > Meinersbur wrote:
> > > [style] No almost-always-auto
> > Fixed, thanks!
> Can make the `&Reference[NumArrayElements / 2]` change too? Its the exact same as `&Reference[0] + NumArrayElements / 2`. `&Reference[0]` is only a more convoluted way to say  `Reference.get()`.
Missed that originally, update, thanks!


Repository:
  rT test-suite

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

https://reviews.llvm.org/D119121