[PATCH] D33750: CGCleanup: (NFC) add a test that used to trigger broken IR

[Gor Nishanov via cfe-commits]

Sure thing. I'll expand the test to verify that frontend emit expected IR
for this case.
Thank you for the feedback

On Mon, Jun 5, 2017 at 9:58 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Wed, May 31, 2017 at 5:45 PM Gor Nishanov via Phabricator via
> cfe-commits <cfe-commits at lists.llvm.org> wrote:
>
>> GorNishanov created this revision.
>>
>> Coroutine related test that used to trigger broken IR prior to r304335.
>>
>>
>> https://reviews.llvm.org/D33750
>>
>> Files:
>>   test/CodeGenCoroutines/coro-await-domination.cpp
>>
>>
>> Index: test/CodeGenCoroutines/coro-await-domination.cpp
>> ===================================================================
>> --- /dev/null
>> +++ test/CodeGenCoroutines/coro-await-domination.cpp
>> @@ -0,0 +1,38 @@
>> +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts
>> -std=c++14 -emit-llvm %s -o - | FileCheck %s
>
> +#include "Inputs/coroutine.h"
>> +
>> +using namespace std::experimental;
>> +
>> +struct coro {
>> +  struct promise_type {
>> +    coro get_return_object();
>> +    suspend_never initial_suspend();
>> +    suspend_never final_suspend();
>> +    void return_void();
>> +    static void unhandled_exception();
>> +  };
>> +};
>> +
>> +struct A {
>> +  ~A();
>> +  bool await_ready();
>> +  int await_resume() { return 8; }
>> +  template <typename F> void await_suspend(F);
>> +};
>> +
>> +extern "C" void consume(int);
>> +
>> +// Verifies that domination is properly built during cleanup.
>> +// Without CGCleanup.cpp fix verifier was reporting:
>> +// Instruction does not dominate all uses!
>> +//  %tmp.exprcleanup = alloca i32*, align 8
>> +//  store i32* %x, i32** %tmp.exprcleanup, align 8
>> +
>> +
>> +// CHECK-LABEL: f(
>>
>
> This doesn't seem to check much. Should this test check that the IR
> instructions have the 'good' layout, more than that the verifier doesn't
> fail?
>
> Or is that already tested by the test case added in r304335? In that case
> I wouldn't add this test. If the change can/is tested in isolation in
> Clang, that should be sufficient/tends to be how testing is done in the
> regression test suites in the LLVM project. (the test-suite is the place
> for broader/end-to-end testing)
>
>
>> +extern "C" coro f(int) {
>> +  int x = 42;
>> +  x = co_await A{};
>> +  consume(x);
>> +}
>> +
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170607/388ee7f2/attachment.html>