[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