Re: [avr-llvm-devel] Regression tests

[Borja F. <bor...@gm...>]

Ok great!

One more thing about the icall test, it can be further reduced because:
  %1 = alloca i32 (i32)*
  store i32 (i32)* %foo, i32 (i32)** %1
  %2 = load i32 (i32)** %1

all that code is redundant, you're getting it because you've ran it with
-O0. If you think about it, you've allocated an i32 var in the stack just
to copy the value of foo but then it's not used at all. This code is
typical from unoptimized builds, so you can reduce it to something like:
  %3 = call i32 %foo(i32 3335977107)
  %4 = add nsw i32 %3, 5

here you're using the foo incoming arg directly.
So as a recommendation, I would also run clang with -03 to get optimized
outputs that are usually clean of redundant allocas.

2012/6/18 Nicklas Bo Jensen <nbj...@gm...>

> I've committed the updated tests, excluding mem.ll awaiting your feedback
> on it (Besides not using immediate 0).
>
> On Mon, Jun 18, 2012 at 1:10 AM, Borja Ferrer <bor...@gm...>wrote:
>
>> return test:
>> 1) you can remove the @return8_reg() test since it's completely commented
>> out.
>> 2) in return64_arg2 you're always going to push r29:r28 as we disccused
>> in a previous email, so you can remove the SPLOW/HI vars, also SP is going
>> to be always the Y reg pair. Using Z here would be a bug.
>> 3) in return64_trunc same as in point 2.
>> 4) add a :TODO: test returning byval structs.
>>
>> call test:
>> 1) in the icall test, the code you're testing is a bit fragile. Basically
>> if you run it with -O3 it will get optimized away and you wont get an
>> icall. Instead, generate the icall using a function pointer from an
>> incoming argument.
>> 2) do the same as in 1 but passing an argument to the icall (any data
>> type will do). (remember i had to fix this last week, so testing it is
>> imporant in case it breaks again in the future)
>> 3) do the same as in 1 using the return value of the icall (any data type
>> will do).
>>
>> One general thing concerning all written tests up to now, can you change
>> all imm values (including the ones already commited), so that you don't
>> test things like "ldi reg, 0". So for a 16bit type instead of using a 5 use
>> a value were none of the registers are zero like 0xABCD instead of 0x0005
>> where the hi part is 0. In the future we'll add an optimization that relies
>> on r0 having a 0, so many tests will break because of this since we could
>> get a mov reg, r0 instead of an ldi.
>>
>> I'll reply tomorrow with comments about the memory tests.
>>
>>
>> 2012/6/17 Borja Ferrer <bor...@gm...>
>>
>>> Thanks, I'm reviewing them now
>>>
>>> El viernes, 15 de junio de 2012, Nicklas Bo Jensen escribió:
>>>
>>> Hi,
>>>>
>>>> Please review these tests.
>>>>
>>>> Any comments?
>>>>
>>>> On Wed, Jun 6, 2012 at 2:46 AM, Borja Ferrer <bor...@gm...>wrote:
>>>>
>>>> Fixed in SVN, let me know any other issues. Thanks for reporting it!
>>>>
>>>>
>>>> 2012/6/5 Borja Ferrer <bor...@gm...>
>>>>
>>>> Something has changed in llvm or maybe it never worked, can't remember,
>>>> but I'm unable to pass arguments through an icall, I'm investigating for a
>>>> fix. In the meantime you can disable that assertion until I come with a
>>>> proper fix.
>>>>
>>>>
>>>> 2012/6/4 Borja Ferrer <bor...@gm...>
>>>>
>>>> I'll take a look to it shortly.
>>>>
>>>>
>>>> 2012/6/3 Nicklas Bo Jensen <nbj...@gm...>
>>>>
>>>> Ok. Sure your explanations are really clear :) Thanks!
>>>>
>>>> I'm having some trouble testing icall, which i'm doing using a function
>>>> pointer. I've attached a small C example, the corresponding cleaned up llvm
>>>> code and the output of running llc on it. There is a failed assertion.
>>>>
>>>> Any thoughts?
>>>>
>>>> On Fri, Jun 1, 2012 at 7:04 PM, Borja Ferrer <bor...@gm...>wrote:
>>>>
>>>> Just checking for the sub instructions that allocate and restore the
>>>> stack should be enough, also check that the allocated size is correct, the
>>>> rest of stuff can be omitted. I would also check the whole sequence
>>>> including the interrupt instructions in a single test, no need to do it
>>>> everywhere. When we get to test allocas we'll have to test for prologue and
>>>> epilogue code, which is very similar to the one you're seeing here.
>>>> About testing push and pops: no need to test that, it's part of the
>>>> prologue/epilogue of the function, so it's not related to calls, also, that
>>>> is handled by llvm so I wouldn't test that at all. Remember to test the
>>>> icall instruction, as far as I remember it needs to be tweaked because for
>>>> now it doesn't save the registers between calls like regular calls do.
>>>>
>>>> Ok, memory instructions, most of the instructions you see in that file
>>>> that end up with a "w" are pseudo instructions (excluding sbiw/adiw, movw
>>>> or other real ones that work with 16bit data) meaning that they dont get
>>>> into the final assembly since they don't exist in the instruction set. It's
>>>> a trick to make llvm think they're supported instructions that way, movws
>>>> and the real 16bit insts get generated, however these pseudo ops get
>>>> expanded into 8bit ops, that's why you wont never see those in the final
>>>> assembly, and if you ever see one then it's a bug. So if you do an and with
>>>> 16bit types, llvm selects the andw pseudo op, and a later pass expands it
>>>> into two real 8bit and insts that are supported by the avr inst set, dont'
>>>> know if this makes things clear.
>>>>
>>>>
>>>> 2012/5/31 Nicklas Bo Jensen <nbj...@gm...>
>>>>
>>>> Checking for stack allocation and restore could be something like:
>>>>
>>>> ; CHECK: in [[REG1:r[0-9]+]], 61
>>>> ; CHECK: in [[REG2:r[0-9]+]], 62
>>>> ; CHECK: sbiw [[REG2]]:[[REG1]], 4
>>>> ; CHECK: out 62, [[REG2]]
>>>> ; CHECK: out 61, [[REG1]]
>>>> ; ... other tests
>>>> ; CHECK: in [[REG1:r[0-9]+]], 61
>>>> ; CHECK: in [[REG2:r[0-9]+]], 62
>>>> ; CHECK: subi [[REG1]], 254
>>>> ; CHECK: sbci [[REG2]], 255
>>>> ; CHECK: out 62, [[REG2]]
>>>> ; CHECK: out 61, [[REG1]]
>>>>
>>>> Is it something like that you have in mind? Should disabling and
>>>> enabling interupts (cli+writeback of SREG) also be tested?
>>>>
>>>> Checkin
>>>>
>>>>
>>
>