From: Nicklas Bo J. <nbj...@gm...> - 2012-07-07 20:18:25
|
That sounds good for std/ldd. Regarding postinc/predec I think the generated code is too fragile to test good, but I will look further into if it's possible to find a better test. On Sat, Jul 7, 2012 at 9:59 PM, Borja Ferrer <bor...@gm...> wrote: > 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 >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> > |