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

[Roman Lebedev via llvm-commits]

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.

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