[PATCH] D78609: [llvm] [X86] Add "REQUIRES asserts" to test
Aart Bik via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 21 22:40:46 PDT 2020
aartbik added a comment.
In D78609#1996030 <https://reviews.llvm.org/D78609#1996030>, @craig.topper wrote:
> In D78609#1995999 <https://reviews.llvm.org/D78609#1995999>, @mehdi_amini wrote:
>
> > I don't really understand why? Can you add the motivation in the commit/revision description?
>
>
> I think because the test is checking the -debug output.
>
> Can we just check the assembly output and trust that asserts builds will trigger the assert that found this originally?
I just found out that this is needed for debug builds. Otherwise the test will fail for optimized builds.
I took inspiration from other files in the same directory, see e.g. merge-store-partially-alias-loads.ll
In this case, testing the assembly would of course work, but it would not be a strict as I wanted it to be.
Given that other tests do exactly this, I figured this was the best-practice for debug output testing.
If there are other best-practices for this, happy to comply...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78609/new/
https://reviews.llvm.org/D78609
More information about the llvm-commits
mailing list