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

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 409676.
fhahn marked 3 inline comments as done.
fhahn added a comment.

Address latest comments, thanks!

In D119121#3327683 <https://reviews.llvm.org/D119121#3327683>, @Meinersbur wrote:

> [suggestion] I am still not convinced `__attribute__((noinline))` is useful and give the wrong impression that its semantics solves the problem. What should not be inlined is the `VectorFn` lambda. Is it possible to attach a noinline attribute to it? Or maybe only call it indirectly through a `__attribute__((optnone))` function. As a suggestion, I will leave it up to you whether you think it is worth it.

Updated to call the vector functions through a optnone function :)

> In D119121#3312322 <https://reviews.llvm.org/D119121#3312322>, @fhahn wrote:
>
>>> [not a change request] If we vary interleaving as well, we would not be restricted to the target architecture's vector width.
>>
>> I was wondering whether we should rely on the vectorizer choosing vectorization factors and interleave counts automatically or if we should force them instead. My reasoning for letting the compiler chose is that we get different combinations for different targets, possibly increasing coverage overall. We could chose the VF automatically and cover a range of user-provided interleave counts ?
>
> [not a change request] My reasoning was to force range of VFs (or interleave count since it does not require a specific instruction set), so the test covers all possible VFs without requiring access to platforms for each of them to test. The native choice by LoopVectorize can still be tested in addition to that, in case the overlap test makes a difference.

Yeah, I think that might be worth as follow-up.

> In D119121#3325503 <https://reviews.llvm.org/D119121#3325503>, @dmgreen wrote:
>
>> I tried copying this into a godbolt link, and is suggest you may need to `#include <functional>` under some systems. There are also some warnings that might be worth cleaning up: https://godbolt.org/z/sMTjfKbGo
>
> gcc indeed does not compile it: https://godbolt.org/z/3acnP796E It would be nice to keep the test-suite buildable with gcc as a comparison/reference.

Thanks, I added the missing include (probably a slight libc++/libstdc++ difference).


Repository:
  rT test-suite

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

https://reviews.llvm.org/D119121

Files:
  SingleSource/UnitTests/Vectorizer/CMakeLists.txt
  SingleSource/UnitTests/Vectorizer/runtime-checks.cpp
  SingleSource/UnitTests/Vectorizer/runtime-checks.reference_output

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D119121.409676.patch
Type: text/x-patch
Size: 12405 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220217/b1aea088/attachment.bin>