[llvm] r262440 - revert r262424 because there's a *clang test* for AArch64 that checks -O3 asm output

[Eric Christopher via llvm-commits]

On Wed, Mar 2, 2016 at 7:54 AM Sanjay Patel <spatel at rotateright.com> wrote:

> Hi Renato -
>
> Thanks for replying. I was going to ping the AArch64 experts about this.
> There were 7 tests that broke with my patch in:
> clang/test/CodeGen/aarch64-neon-misc.c
>
> The failures are as expected from what I can tell: my instcombine change
> would transform vector compare+sext into shifts. Note that this is an
> existing transform, and I'm trying to extend it to catch the cases seen in
> PR26701.
>
> This is at least the 3rd time I've had a clang test fail on me after
> making an LLVM optimizer change. So I admit that I wanted to just XFAIL the
> whole thing, but I wouldn't do that. :)
>
> The last time I hit this kind of problem, I left a FIXME:
> http://reviews.llvm.org/rL256395
>
> Regarding the validity of the test file itself - there was strong
> indication that we shouldn't have end-to-end regression tests (even with
> -O0) here:
>
> http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20151123/143974.html
> (cc'd Eric and Simon in case I missed anything after that).
>
> So unless something has changed, I think we need to split each test in
> aarch64-neon-misc.c (and other files like it) into pieces:
> 1. A clang test for the C-level intrinsics -> LLVM intrinsics and/or
> native IR.
> 2. A codegen test for LLVM intrinsic/IR -> asm.
>
>
Yep yep yep.

This is exactly why I say that -O3 tests shouldn't belong in clang and the
fact this has happened 3 times to Sanjay just emphasizes the point.

-eric


>
>
> On Wed, Mar 2, 2016 at 2:05 AM, Renato Golin <renato.golin at linaro.org>
> wrote:
>
>> On 2 March 2016 at 01:04, Sanjay Patel via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>> > Author: spatel
>> > Date: Tue Mar  1 19:04:09 2016
>> > New Revision: 262440
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=262440&view=rev
>> > Log:
>> > revert r262424 because there's a *clang test* for AArch64 that checks
>> -O3 asm output
>> > that is broken by this change
>>
>> The question is whether the test caught something worthy or not.
>>
>> I'm not discussing the validity of such tests, just that we may need
>> to move it down to LLVM with care, rather than ignore or remove the
>> test.
>>
>> Which test was it and how did it fail?
>>
>> cheers,
>> --renato
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160302/d19b62d9/attachment.html>