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

[Nikita Popov via llvm-commits]

On Mon, May 3, 2021 at 8:40 AM Roman Lebedev via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Doesn't verifier run on the fully constructed IR?
> I.e. won't it always run after said constructor has run?
> I'm not really sure how we can perform that verifier check earlier?
>
> Roman
>

I think you need to either drop the assert, or check this condition already
in LLParser, maybe somewhere around
https://github.com/llvm/llvm-project/blob/1f8963c80195aa86d02e81acedcf1ff3da127842/llvm/lib/AsmParser/LLParser.cpp#L7175.
And possibly the Bitcode parser as well?

Nikita


> On Sun, May 2, 2021 at 10:46 PM David Blaikie <dblaikie at gmail.com> wrote:
> >
> > 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
> _______________________________________________
> 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/20210503/18a40001/attachment.html>