Re: [avr-llvm-devel] Regression tests

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

No problem, I just wanted to know what was the status, no need to rush it :)

2012/7/7 Nicklas Bo Jensen <nbj...@gm...>

> No, sorry, I've been busy lately. I will take it up again in the next days.
>
>
> On Sat, Jul 7, 2012 at 6:21 PM, Borja Ferrer <bor...@gm...>wrote:
>
>> Nicklas, are you still working in those memory tests?
>>
>> 2012/6/26 Borja Ferrer <bor...@gm...>
>>
>>> What about directmem.ll to keep it a bit shorter?
>>>
>>> Btw, the ldd/std tests and such keep them in two separate files, like
>>> load.ll and store.ll since they're going to get quite big on their own.
>>>
>>>
>>> 2012/6/25 Nicklas Bo Jensen <nbj...@gm...>
>>>
>>>> I've commited the tests after going over your comments.
>>>>
>>>> The file should probably still be renamed though, we could call it
>>>> loadstoredirect.ll? Then tests for st/std/ld/ldd could be in a file called
>>>> loadstoreindirect.ll?
>>>>
>>>> On Mon, Jun 25, 2012 at 1:43 PM, Borja Ferrer <bor...@gm...>wrote:
>>>>
>>>>> Ohhh well, happy to hear it's now fixed!! Now we can move to something
>>>>> else.
>>>>>
>>>>> Memory test comments:
>>>>> First of all since you're testing lds/sts i would rename the file to
>>>>> something else, mem is too general, but i can't think of a good name now,
>>>>> suggestions are welcome.
>>>>> 1) I would remove the allocX_local tests, I'm writing alloca tests
>>>>> somewhere else, and they're unrelated to sts/lds.
>>>>> 2) Change all imm values to something that doesn't contain zeros, as
>>>>> we mentioned previously. This will change some tests a bit, but it's
>>>>> trivial to implement.
>>>>>
>>>>> Apart of these comments, the tests look great.
>>>>>
>>>>> Once this gets done we can then move to other memory tests, mainly
>>>>> testing st/ld and std/ldd instructions. Which require testing displacements
>>>>> of small and big offsets, postinc and predec operations, and probably other
>>>>> stuff that I'll have to look up.
>>>>>
>>>>>
>>>>>
>>>>> 2012/6/25 Nicklas Bo Jensen <nbj...@gm...>
>>>>>
>>>>>> OK, it's working now. I was missing MyFlags.Flags.setSplitPiece(j) in
>>>>>> line 6450 in selectiondagbuilder.cpp. Not sure how it 'unpatched' itself.
>>>>>>
>>>>>> Sorry for the trouble.
>>>>>>
>>>>>>
>>>>>> On Mon, Jun 25, 2012 at 12:40 AM, Borja Ferrer <bor...@gm...
>>>>>> > wrote:
>>>>>>
>>>>>>> IIRC CLI.Outs.size() should be 4 because you have 4 argument pieces,
>>>>>>> however ParseExternFuncCallArgs should have filled the Args array to have
>>>>>>> only 1 element of size 4 because the function in reality has only 1
>>>>>>> argument. The clue here is investigating why is this failing for you and
>>>>>>> not for me. I guess you're jumping to the AnalyzeArguments from line 932,
>>>>>>> the one in the else clause.
>>>>>>>
>>>>>>> Add printf's to see what getSplitPiece returns for each iteration,
>>>>>>> in theory assuming this wrong behaviour this function should always return
>>>>>>> 0 so when doing Out.push_back(Size), Size is always 1.
>>>>>>> The splitpiece stuff is set by MyFlags.Flags.setSplitPiece(j) in
>>>>>>> line 6450 in selectiondagbuilder.cpp
>>>>>>>
>>>>>>>
>>>>>>> 2012/6/24 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>
>>>>>>>> Yes, I have four elements with value one filled
>>>>>>>> by ParseExternFuncCallArgs.
>>>>>>>>
>>>>>>>> I can't tell if ParseExternFuncCallArgs is buggy, but it seems as
>>>>>>>> though it is given that there are four arguments. For example
>>>>>>>> CLI.Outs.size() evaluates to 4 in line 893. Is that correctly understood?
>>>>>>>>
>>>>>>>> On Sun, Jun 24, 2012 at 9:57 PM, Borja Ferrer <
>>>>>>>> bor...@gm...> wrote:
>>>>>>>>
>>>>>>>>> Indeed, if you mean the Size var set in line 750 it should be 4.
>>>>>>>>> The Args vector should only contain 1 element because you only have 1
>>>>>>>>> argument in the function and the size should be set to 4. This is the
>>>>>>>>> reason of why the arguments aren't reversed. In your case I guess you have
>>>>>>>>> 4 elements of size 1. This vector is filled by ParseExternFuncCallArgs() or
>>>>>>>>> by ParseFunctionArgs() depending on the call type, so the problem is there,
>>>>>>>>> in this case ParseExternFuncCallArgs is doing the work right? So this is
>>>>>>>>> the function you should debug.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 2012/6/24 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>>>
>>>>>>>>>> You're right, it's the -O3 flag.
>>>>>>>>>>
>>>>>>>>>> I'm stepping through the code with the debugger right now. The
>>>>>>>>>> indirect call does trigger AnalyzeArguments to reach line 768
>>>>>>>>>> (std::reverse).
>>>>>>>>>>
>>>>>>>>>> There is one thing puzzling me a bit, the argument size is
>>>>>>>>>> calculated to be 1, i.e. Args[0] is 1, but shouldn't a 64bit argument
>>>>>>>>>> evaluate to 4 in size?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Sun, Jun 24, 2012 at 8:16 PM, Borja Ferrer <
>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>
>>>>>>>>>>> With that command line I get the same code as you (now i get the
>>>>>>>>>>> spill i wasn't getting before), however i still get correct code:
>>>>>>>>>>>     movw    r31:r30, r25:r24
>>>>>>>>>>>
>>>>>>>>>>>     std    Y+1, r30
>>>>>>>>>>>     std    Y+2, r31
>>>>>>>>>>>     ldi    r18, 136
>>>>>>>>>>>     ldi    r19, 153
>>>>>>>>>>>     ldi    r20, 255
>>>>>>>>>>>     ldi    r21, 238
>>>>>>>>>>>     ldi    r22, 221
>>>>>>>>>>>     ldi    r23, 204
>>>>>>>>>>>     ldi    r24, 187
>>>>>>>>>>>     ldi    r25, 170
>>>>>>>>>>>     icall
>>>>>>>>>>>
>>>>>>>>>>> This is my usual command line:
>>>>>>>>>>>
>>>>>>>>>>> clang -O3 -emit-llvm test.c -S -o test.bc -ccc-host-triple
>>>>>>>>>>> avr-generic-generic -ccc-clang-archs avr
>>>>>>>>>>> llc -O3 -march=avr test.bc -pre-RA-sched=default
>>>>>>>>>>> -regalloc=avrgreedy -debug-only=isel -o test.S -print-isel-input
>>>>>>>>>>> -print-after-all -stats 2> dump
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> 2012/6/24 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>>>>>
>>>>>>>>>>>> All patches should be installed and it has worked earlier when
>>>>>>>>>>>> I wrote that test. I also have the clang patches, but I don't think it has
>>>>>>>>>>>> anything to do with those.
>>>>>>>>>>>>
>>>>>>>>>>>> To compile icall.c, I would for example do:
>>>>>>>>>>>>
>>>>>>>>>>>> ../build/Debug+Asserts/bin/clang icall.c -S -emit-llvm -o
>>>>>>>>>>>> icall.ll
>>>>>>>>>>>>
>>>>>>>>>>>> to view the llvm ir and to get assembler:
>>>>>>>>>>>>
>>>>>>>>>>>> ../build/Debug+Asserts/bin/llc icall.ll -march=avr -O3 -o
>>>>>>>>>>>> icall.s
>>>>>>>>>>>>
>>>>>>>>>>>> I have that exact code as you describe, I will printf debug it
>>>>>>>>>>>> later tonight.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Sun, Jun 24, 2012 at 1:58 PM, Borja Ferrer <
>>>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Ok the problem I see is that you're not getting the arguments
>>>>>>>>>>>>> pieces reversed. Can you check the code that performs this? This is done
>>>>>>>>>>>>> inside AnalyzeArguments, exactly in line 768 done by std::reverse. Also
>>>>>>>>>>>>> check that this function is being called in line 932. That should give you
>>>>>>>>>>>>> enough info to debug it, use fprintfs to print out debugging info to help
>>>>>>>>>>>>> you otherwise it's going to be hard to debug.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I'm assuming you have all patches installed, including clang
>>>>>>>>>>>>> patches right?
>>>>>>>>>>>>> Paste the command line you're using to call clang because I'm
>>>>>>>>>>>>> not getting
>>>>>>>>>>>>>     std    Y+1, r30
>>>>>>>>>>>>>     std    Y+2, r31
>>>>>>>>>>>>> or any stack allocation as you get.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> 2012/6/24 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Still gives the wrong code:
>>>>>>>>>>>>>> ldi r24, 136
>>>>>>>>>>>>>> ldi r25, 153
>>>>>>>>>>>>>>  ldi r22, 255
>>>>>>>>>>>>>> ldi r23, 238
>>>>>>>>>>>>>>  ldi r20, 221
>>>>>>>>>>>>>> ldi r21, 204
>>>>>>>>>>>>>>  ldi r18, 187
>>>>>>>>>>>>>> ldi r19, 170
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I have also attached the output.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Sun, Jun 24, 2012 at 1:38 AM, Borja Ferrer <
>>>>>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> What do you get with the following code?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> long icall(long (*x)(long long))
>>>>>>>>>>>>>>> {
>>>>>>>>>>>>>>>   return x(0xAABBCCDDEEFF9988LL)+5;
>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> John could you check if you're getting the right code?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 2012/6/24 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> No, regular calls/and/xor/or works fine with 3335977107.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Sun, Jun 24, 2012 at 12:45 AM, Borja Ferrer <
>>>>>>>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> This is very weird. Do you get this problem with other
>>>>>>>>>>>>>>>>> code when you use this particular number (3335977107) like with a regular
>>>>>>>>>>>>>>>>> call or with an and/xor/or?
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> 2012/6/24 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I also get the wrong result compiling that snippet...
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I have tried to revert the last patches, those that not
>>>>>>>>>>>>>>>>>> needed for syncing with the llvm repo, same result. I have also updated
>>>>>>>>>>>>>>>>>> llvm. Same result.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Please let me know if you have any ideas what it could be
>>>>>>>>>>>>>>>>>> :)
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On Sat, Jun 23, 2012 at 8:15 PM, Borja Ferrer <
>>>>>>>>>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Thanks for the patch there!
>>>>>>>>>>>>>>>>>>> Very interesting because I'm not getting that error in
>>>>>>>>>>>>>>>>>>> icall at all, I'm getting the second code snippet you pasted.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> With this code:
>>>>>>>>>>>>>>>>>>> long icall(long (*x)(long))
>>>>>>>>>>>>>>>>>>> {
>>>>>>>>>>>>>>>>>>>   return x(0xC6D6F893)+5;
>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I get:
>>>>>>>>>>>>>>>>>>>     movw    r19:r18, r25:r24
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>     ldi    r22, 147
>>>>>>>>>>>>>>>>>>>     ldi    r23, 248
>>>>>>>>>>>>>>>>>>>     ldi    r24, 214
>>>>>>>>>>>>>>>>>>>     ldi    r25, 198
>>>>>>>>>>>>>>>>>>>     movw    r31:r30, r19:r18
>>>>>>>>>>>>>>>>>>>     icall
>>>>>>>>>>>>>>>>>>>     subi    r22, 251
>>>>>>>>>>>>>>>>>>>     sbci    r23, 255
>>>>>>>>>>>>>>>>>>>     sbci    r24, 255
>>>>>>>>>>>>>>>>>>>     sbci    r25, 255
>>>>>>>>>>>>>>>>>>>     ret
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Can you re check to see what's going on?
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> The comments in zext.ll and sext.ll are there for
>>>>>>>>>>>>>>>>>>> reference, they cover the cases implemented in the pseudo instruction
>>>>>>>>>>>>>>>>>>> expander, that way we're able to know what test covers each case.
>>>>>>>>>>>>>>>>>>> Yes i have comments for the memory tests, I'll write
>>>>>>>>>>>>>>>>>>> them in the next email. We should continue with memory operations, there
>>>>>>>>>>>>>>>>>>> are many more things to test there that i'll list next time. For now we
>>>>>>>>>>>>>>>>>>> have to see what's going on with that failing test.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> 2012/6/23 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> I've committed the small change in Targets.cpp as you
>>>>>>>>>>>>>>>>>>>> described.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> We are breaking a test now. In call.s, icall is loading
>>>>>>>>>>>>>>>>>>>> arguments incorrectly now.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> The instruction %1 = call i32 %foo(i32 3335977107)
>>>>>>>>>>>>>>>>>>>>  gives:
>>>>>>>>>>>>>>>>>>>> ldi r24, 147
>>>>>>>>>>>>>>>>>>>> ldi r25, 248
>>>>>>>>>>>>>>>>>>>>  ldi r22, 214
>>>>>>>>>>>>>>>>>>>> ldi r23, 198
>>>>>>>>>>>>>>>>>>>> but should give:
>>>>>>>>>>>>>>>>>>>> ldi r22, 147
>>>>>>>>>>>>>>>>>>>> ldi r23, 248
>>>>>>>>>>>>>>>>>>>> ldi r24, 214
>>>>>>>>>>>>>>>>>>>>  ldi r25, 198
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Also in your new sext.ll and zext.ll tests there are
>>>>>>>>>>>>>>>>>>>> some comments above each function, should these be deleted?
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Last did you have any comments above the memory tests?
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Any preferences for what I should continue on?
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> On Tue, Jun 19, 2012 at 1:47 PM, Borja Ferrer <
>>>>>>>>>>>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Nicklas, in the meantime while the patch gets fixed
>>>>>>>>>>>>>>>>>>>>> you can manually fix the error by adding that virtual BuiltinVaListKind
>>>>>>>>>>>>>>>>>>>>> getBuiltinVaListKind() const function in the AVR class, in the above link
>>>>>>>>>>>>>>>>>>>>> you have an example on how to do it.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> 2012/6/19 Borja Ferrer <bor...@gm...>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> John can you fix this issue, i have a different local
>>>>>>>>>>>>>>>>>>>>>> copy of Targets.cpp that i don't want to change now with the patch in SVN.
>>>>>>>>>>>>>>>>>>>>>> The fix is in this commit
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?r1=158334&r2=158592&sortby=date&diff_format=h
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> so updating the patch file will do it.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> 2012/6/18 Borja Ferrer <bor...@gm...>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> You've done it right, the problem is here:
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?r1=158334&r2=158592&sortby=date&diff_format=h
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> it's caused by a recent API change that has broken
>>>>>>>>>>>>>>>>>>>>>>> out of tree targets, i'll fix it soon.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> 2012/6/18 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Yes of course.
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> I'm having some problems compiling getting the
>>>>>>>>>>>>>>>>>>>>>>>> following error:
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> /avr-llvm/llvm/tools/clang/lib/Basic/Targets.cpp:4172:16:
>>>>>>>>>>>>>>>>>>>>>>>> error: allocating an object of
>>>>>>>>>>>>>>>>>>>>>>>>       abstract class type 'AVRTargetInfo'
>>>>>>>>>>>>>>>>>>>>>>>>     return new AVRTargetInfo(T);
>>>>>>>>>>>>>>>>>>>>>>>>                ^
>>>>>>>>>>>>>>>>>>>>>>>> /avr-llvm/llvm/tools/clang/lib/Basic/../../include/clang/Basic/TargetInfo.h:406:29:
>>>>>>>>>>>>>>>>>>>>>>>> note:
>>>>>>>>>>>>>>>>>>>>>>>>       unimplemented pure virtual method
>>>>>>>>>>>>>>>>>>>>>>>> 'getBuiltinVaListKind' in 'AVRTargetInfo'
>>>>>>>>>>>>>>>>>>>>>>>>   virtual BuiltinVaListKind getBuiltinVaListKind()
>>>>>>>>>>>>>>>>>>>>>>>> const = 0;
>>>>>>>>>>>>>>>>>>>>>>>>                             ^
>>>>>>>>>>>>>>>>>>>>>>>>  1 error generated.
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> I think I have applied the patches from svn
>>>>>>>>>>>>>>>>>>>>>>>> correctly, at least did it a few times. I'm not stuck, but any ideas would
>>>>>>>>>>>>>>>>>>>>>>>> be nice.
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> On Mon, Jun 18, 2012 at 1:46 PM, Borja Ferrer <
>>>>>>>>>>>>>>>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> 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
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>