[llvm-commits] [llvm] r67635 - in /llvm/trunk: lib/Transforms/Scalar/InstructionCombining.cpp test/Transforms/InstCombine/shift.ll

[Misha Brukman]

On Tue, Mar 24, 2009 at 10:09 PM, Chris Lattner <clattner at apple.com> wrote:

> On Mar 24, 2009, at 2:45 PM, Misha Brukman wrote:
>
> On Tue, Mar 24, 2009 at 2:15 PM, Chris Lattner <sabre at nondot.org> wrote:
>
>>
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/InstCombine/shift.ll (original)
>> +++ llvm/trunk/test/Transforms/InstCombine/shift.ll Tue Mar 24 13:15:30
>> 2009
>> @@ -206,4 +206,11 @@
>>         %D = shl i32 %C, 1              ; <i32> [#uses=1]
>>         ret i32 %D
>>  }
>> +
>> +
>> +define i1 @test27(i32 %x) nounwind {
>> +  %y = lshr i32 %x, 3
>> +  %z = trunc i32 %y to i1
>> +  ret i1 %z
>> +}
>
>
> This would work great as a unittest.  Just sayin'.
>
> Why would it be better as a unit test?
>

shift.ll has this RUN line:
 ; RUN: llvm-as < %s | opt -instcombine | llvm-dis | not grep sh
which means that all -instcombine needs to do is to delete any shift-related
instructions and it would still pass.  This test is not checking that
-instcombine is replacing each shift instruction with the proper sequence of
instructions, which you cannot do with grep.  Also, to test this .ll file,
it requires loading and running 5 executables (llvm-{as,dis}, opt, grep, and
bash for "not"), and parsing/unparsing LLVM assembly and bytecode, the
latter of which are orthogonal to the thing being tested, but still take
time.  The test suite then does this over and over for each .ll file being
tested.

To improve the test you *could* replace the "not grep sh" with a diff to a
"golden file" that has all the expected code sequences, but I would argue
that we can make the tests run faster by avoiding all the extra binary
invocations (and in the latter case, also writing to a file on disk and
calling diff on the two) and just write a unittest instead.

Misha
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20090325/eaa93f9d/attachment.html>