[llvm] r205484 - Add test case for [Constant Hoisting] Erase dead cast instructions (r204538).

David Blaikie dblaikie at gmail.com
Thu Apr 3 09:29:45 PDT 2014


On Thu, Apr 3, 2014 at 9:10 AM, Juergen Ributzka <juergen at apple.com> wrote:
> Unfortunately there is nothing observable in the IR (like an extra
> instruction) I could have tested for.
>
> In this particular case I removed a no longer needed instruction, because it
> had an empty use list.
> The instruction was only removed, but not deleted. Later on when the
> destructor is called it hit an
> assertion.
>
> I will change the test to limit is to assert builds, as suggested by Eric,
> and check for the expected
> output.
>
> -Juergen
>
> On Apr 3, 2014, at 8:25 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
> For that matter - tests that simply test that we don't crash always
> seem rather suspect. What is the behavior we expect that we weren't
> getting due to the crash being in the way? We should really test for
> that.

I suppose what I mean is that the latter (checking for the expected
output) is a test case we needed (whatever codepath this was going
down to crash was not being exercised even if it wasn't directly
observable, I assume the code had /some/ effect or we wouldn't be
running it). Just leaving that test case in, checking the output, in
all (even non-asserts) builds seems appropriate.

- David



More information about the llvm-commits mailing list