[llvm] b01f499 - [NFC][Verifier] Split token1.ll into two, assert/non-assert versions

[David Blaikie via llvm-commits]

On Sun, May 2, 2021 at 12:34 PM Roman Lebedev <lebedev.ri at gmail.com> wrote:

> Originally we had a verifier check for phis with token type,
> i made the PHINode constructor assert if the type is a token,
> which obviously broke the verifier test.
>

Sounds like the verifier may need to be changed to check this condition
earlier, then? We shouldn't be able to reach/fire an assertion on any input
when running the verifier, I think.


> If we drop the test for what happens without assertions,
> that implies we no longer care what the verifier does
> about such PHI's. Is that really so?
>
> Roman
>
> On Sun, May 2, 2021 at 10:11 PM David Blaikie <dblaikie at gmail.com> wrote:
> >
> > Could you explain the purpose of these two tests a bit more?
> >
> > In general I don't think we should be testing the behavior of assertions
> except in pretty narrow cases (like checking that the Error API asserts in
> certain ways that are critical to its usability) - since they represent
> essentially "undefined behavior".
> >
> > And if we are testing the behavior of an assertion, we shouldn't be
> testing what happens without assertions - because, again, if an assertion
> can be triggered then the behavior "behind" the assert is undefined, we
> shouldn't expect any particular behavior. (if we expect particular
> behavior, we shouldn't assert)
> >
> > On Wed, Apr 28, 2021 at 3:59 AM Roman Lebedev via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
> >>
> >>
> >> Author: Roman Lebedev
> >> Date: 2021-04-28T13:58:38+03:00
> >> New Revision: b01f499861235f414f6740bbf950d095bceada4d
> >>
> >> URL:
> https://github.com/llvm/llvm-project/commit/b01f499861235f414f6740bbf950d095bceada4d
> >> DIFF:
> https://github.com/llvm/llvm-project/commit/b01f499861235f414f6740bbf950d095bceada4d.diff
> >>
> >> LOG: [NFC][Verifier] Split token1.ll into two, assert/non-assert
> versions
> >>
> >> Added:
> >>     llvm/test/Verifier/token1-with-asserts.ll
> >>     llvm/test/Verifier/token1-without-asserts.ll
> >>
> >> Modified:
> >>
> >>
> >> Removed:
> >>     llvm/test/Verifier/token1.ll
> >>
> >>
> >>
> ################################################################################
> >> diff  --git a/llvm/test/Verifier/token1.ll
> b/llvm/test/Verifier/token1-with-asserts.ll
> >> similarity index 100%
> >> rename from llvm/test/Verifier/token1.ll
> >> rename to llvm/test/Verifier/token1-with-asserts.ll
> >>
> >> diff  --git a/llvm/test/Verifier/token1-without-asserts.ll
> b/llvm/test/Verifier/token1-without-asserts.ll
> >> new file mode 100644
> >> index 000000000000..ef2d25c045d2
> >> --- /dev/null
> >> +++ b/llvm/test/Verifier/token1-without-asserts.ll
> >> @@ -0,0 +1,12 @@
> >> +; REQUIRES: !asserts
> >> +; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
> >> +
> >> +define void @f(token %A, token %B) {
> >> +entry:
> >> +  br label %bb
> >> +
> >> +bb:
> >> +  %phi = phi token [ %A, %bb ], [ %B, %entry]
> >> +; CHECK: PHI nodes cannot have token type!
> >> +  br label %bb
> >> +}
> >>
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210502/0b4de72c/attachment.html>