Re: [avr-llvm-devel] Regression tests

[Borja F. <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?
>>>
>>> Checking for the push and pops is going to be very fragile I think.
>>>
>>> Other backends have less thorough tests, but a bit alike, e.g. MBlaze in
>>> test/CodeGen/MBlaze/cc.ll. Here they do tests that registers are passed and
>>> returned in the correct registers or passed on the stack if needed.
>>>
>>> I've come quite far with memory operations, and have noticed the
>>> operations stsw and lddw in AVRInstrInfo.td, however I cannot find these in
>>> the Atmel AVR Instruction Set and cannot make llvm generate them. Could you
>>> give me a hint to why this is?
>>>
>>> On Thu, May 31, 2012 at 1:24 PM, Borja Ferrer <bor...@gm...>wrote:
>>>
>>>> They look very good. Yes, the order of loads is an issue, I don't know
>>>> how fragile this can get in case there are future changes that could alter
>>>> the order. Also it could be nice to check for stack allocation and restore,
>>>> have you checked if it's possible to do it? Add a TODO in the file for args
>>>> and for calls for testing for structs, this is something unimplemented that
>>>> needs to gets covered, I don't know yet any conventions for those.
>>>>
>>>> One last thing, what do other backends do to cover these tests?
>>>> Anything similar to what we do?
>>>>
>>>>
>>>>
>>>> 2012/5/30 Nicklas Bo Jensen <nbj...@gm...>
>>>>
>>>>> Thanks for your explanation, it's clear.
>>>>>
>>>>> After a few days on holiday, I've worked on testing the calling part
>>>>> of the calling convention. Please see the attached file.
>>>>>
>>>>> I'm testing only passing parameters in the registers and on the stack.
>>>>> I'ts a bit hard to test in a good way, as for example the order of loads
>>>>> might be different, breaking the tests without having invalid code. I've
>>>>> tried to only tests the essentials in each case. Any comments or ideas for
>>>>> further tests?
>>>>>
>>>>>
>>>>> On Sat, May 26, 2012 at 6:28 PM, Borja Ferrer <bor...@gm...>wrote:
>>>>>
>>>>>> About point 2:
>>>>>> As the calling convention states, arguments that don't fit in
>>>>>> registers r25 down to r8 are passed through the stack, that's why the last
>>>>>> argument is being passed this way. If registers Y or Z are used depends on
>>>>>> a number of conditions that are bit hard to explain, it depends if there
>>>>>> are spilled registers, allocas, and incoming arguments in the stack. In
>>>>>> this particular case, Y is ALWAYS used when reading incoming arguments from
>>>>>> the stack, Z COULD be used only when passing arguments into a function
>>>>>> call. So the difference is that Y is always used to READ incoming arguments
>>>>>> from the stack, and Z or Y can be used to WRITE arguments on the stack as
>>>>>> the preparation of the function call.
>>>>>>
>>>>>> About the code you attached I find it's correct. It's a void function
>>>>>> so why care on computing an add instruction where its result is unused,
>>>>>> also the return value of the called function is unused as well since the
>>>>>> add instruction is useless. That's why you see r25:r24 being overwritten by
>>>>>> the stack pointer so it can be restored to its original state when the
>>>>>> function ends.
>>>>>> This is all a bit tricky so if you dont understand feel free to ask,
>>>>>> it's important to get it right so you can write good tests.
>>>>>>
>>>>>> 2012/5/25 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>
>>>>>>> 1) The i8 @return8_reg test is fully commented out, can it be
>>>>>>>> removed? I don't understand what you want to test here.
>>>>>>>>
>>>>>>> Can be removed, was not supposed to be there :)
>>>>>>>
>>>>>>>
>>>>>>>> 2) In return64_arg2 the backend will always use Y to read arguments
>>>>>>>> from the stack, so there's no need to check for Z.
>>>>>>>>
>>>>>>> Ah ok, is there any rule for putting them on the stack? Here it can
>>>>>>> be both Y and Z right?
>>>>>>>
>>>>>>>
>>>>>>>> About your calling tests, I can't see them xD
>>>>>>>
>>>>>>> Attached :)
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>