There are now quite a few SVE tests in LLVM and Clang that do not
emit warnings related to invalid use of EVT::getVectorNumElements()
and VectorType::getNumElements(). For these tests I have added
additional checks that there are no warnings in order to prevent
any future regressions.
Details
Diff Detail
Event Timeline
I guess we could do this as a temporary measure, if you think it's useful; eventually, of course, the codepath to print the warning will go away.
Since you checked this, how many tests do still print warnings?
Hi @efriedma, at least amongst all the tests in llvm/test/CodeGen/AArch64/sve-* there are still 66 with warnings. @sdesmalen and I discussed this and our reason for adding checks for warnings is mainly to do with the fact we are still fixing up cases and implementing SVE codgen support, so there is a chance that somewhere we'll introduce a regression without realising it. That's the main rationale for this, but I do realise that ultimately getVectorNumElements() will be deprecated. I also realised that the warnings I've added to the clang tests aren't that useful unless we're compiling them to real assembly so I may add some new RUN lines for those.
Okay.
Thinking about it a bit more, I think I'd prefer using a pattern where we redirect stderr to a file, and CHECK it separately, to make sure we don't miss a warning because it got interleaved into the middle of stdout.
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abd.c | ||
---|---|---|
4 | Just thinking out loud here; we don't need to test for the specific warnings, because LLVM doesn't emit any other warnings and any Clang's warnings are otherwise caught by -Werror. | |
4 | Can you add a comment to these checks, so that if someone breaks it (because their code causes a new warning), they know where to look? |
Hi @sdesmalen @efriedma , hopefully I've addressed your review comments with my latest patch!
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adda.c | ||
---|---|---|
3 ↗ | (On Diff #271591) | This needs a -o -, or better a -o /dev/null instead of the piping to /dev/null. As-is, this creates a .s file in the test dir next to this file which on the next run is executed as a test that's unresolved. (Add a rm -f %S/acle_sve_adda.s to clean up bots, with a FIXME to remove this after a few days / weeks. |
Just thinking out loud here; we don't need to test for the specific warnings, because LLVM doesn't emit any other warnings and any Clang's warnings are otherwise caught by -Werror.