Menu
▾
▴
Re: [avr-llvm-devel] Regression tests
|
From: Borja F. <bor...@gm...> - 2012-06-25 11:43:57
|
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
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
|
