From: Borja F. <bor...@gm...> - 2012-07-07 19:59:10
|
Do only one store/load per test function, that way you won't get predec/postinc instructions. What I was thinking of is: test x[0]=value; for every data type, then do the same thing for load. Then test displaced loads/stores with x[44]=value, you cover there std/ldd. And then finally postinc/predec. In fact from the piece of code you pasted, the optimal thing i guess would be doing st X+, reg instead of wasting reg Z, so it could be improved, but i'm not interested now in optimum code. 2012/7/7 Nicklas Bo Jensen <nbj...@gm...> > Of course, but I'm also thinking pre/post dec/inc. > > For example: > > void foo(char *arr) > { > arr[0] = 1; > arr[1] = 2; > arr[2] = 3; > arr[3] = 4; > } > > generates: > movw r31:r30, r25:r24 > ldi r24, 1 > movw r27:r26, r31:r30 > st X+, r24 > ldi r24, 2 > st X, r24 > ldi r24, 3 > std Z+2, r24 > ldi r24, 4 > std Z+3, r24 > ret > > First two stores using st and last two using std. The performance of st vs > std is equivalent right? > > Testing these is going to be hard if i'ts not deterministic which > instruction is used when. > > > > On Sat, Jul 7, 2012 at 9:43 PM, Borja Ferrer <bor...@gm...>wrote: > >> This : >> void test44(int *x) >> { >> x[0]=2; >> } >> gives: >> >> movw r31:r30, r25:r24 >> ldi r24, 2 >> ldi r25, 0 >> st Z, r24 >> std Z+1, r25 >> ret >> >> >> 2012/7/7 Nicklas Bo Jensen <nbj...@gm...> >> >>> Ah thanks! I cannot seem to get ld/st though? >>> >>> >>> On Sat, Jul 7, 2012 at 9:19 PM, Borja Ferrer <bor...@gm...>wrote: >>> >>>> Ok great. If you want that piece of code really working, you can tell >>>> llvm to expand the mul nodes, you can do it in the AVRTargetLowering >>>> constructor with the setOperationAction. >>>> >>>> To generate those instructions it's quite easy, just use a pointer as >>>> an incoming argument in a function and store or load values from there, >>>> then use x[3] and such to get offsets in relation to a base address for >>>> std/ldd. It's important to test offsets in range (up to 63) and bigger ones >>>> which need materialization each time. >>>> >>>> >>>> 2012/7/7 Nicklas Bo Jensen <nbj...@gm...> >>>> >>>>> Nice, that was a quick fix :) I'ts working now with -O0, with -O3 I'm >>>>> getting the mul error. >>>>> >>>>> I mean st/std/ld/ldd. >>>>> >>>>> >>>>> On Sat, Jul 7, 2012 at 8:51 PM, Borja Ferrer <bor...@gm...>wrote: >>>>> >>>>>> Ok fixed, thanks for the report! If you now get the issue about the >>>>>> mul, it's a known thing. >>>>>> >>>>>> What do you mean by indirect mem accesses? >>>>>> >>>>>> >>>>>> 2012/7/7 Nicklas Bo Jensen <nbj...@gm...> >>>>>> >>>>>>> Great, not just my setup then :) >>>>>>> >>>>>>> No, it's not part of any tests. I'm just trying out different >>>>>>> things. I'm having some trouble generating indirect memory accesses, any >>>>>>> tips? >>>>>>> >>>>>>> >>>>>>> On Sat, Jul 7, 2012 at 8:20 PM, Borja Ferrer <bor...@gm...>wrote: >>>>>>> >>>>>>>> Ok, I'm able to reproduce this crash with both clang and llc with >>>>>>>> -O0, with -O3 I'm getting an error because it cant select the mul >>>>>>>> instruction which is a known issue. I'm investigating now. Btw, is this >>>>>>>> part of a test you're writing? >>>>>>>> >>>>>>>> >>>>>>>> 2012/7/7 Nicklas Bo Jensen <nbj...@gm...> >>>>>>>> >>>>>>>>> Could you try to compile the following piece of C: >>>>>>>>> >>>>>>>>> typedef unsigned char uint8_t; >>>>>>>>> >>>>>>>>> extern uint8_t as[4]; >>>>>>>>> extern uint8_t bs[4]; >>>>>>>>> >>>>>>>>> void foo1(void) { >>>>>>>>> for (uint8_t i = 0; i < 4; i++) { >>>>>>>>> bs[i] = as[1]; >>>>>>>>> } >>>>>>>>> } >>>>>>>>> >>>>>>>>> I'm getting some wierd errors with llc(changing a bit dependent on >>>>>>>>> -O0/-O3) like: >>>>>>>>> llvm/include/llvm/CodeGen/SelectionDAGNodes.h:534: const >>>>>>>>> llvm::SDValue &llvm::SDNode::getOperand(unsigned int) const: Assertion `Num >>>>>>>>> < NumOperands && "Invalid child # of SDNode!"' failed. >>>>>>>>> >>>>>>>>> >>>>>>>>> On Sat, Jul 7, 2012 at 6:44 PM, Borja Ferrer < >>>>>>>>> bor...@gm...> wrote: >>>>>>>>> >>>>>>>>>> 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 >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> > |