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 >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> > |