From: Nicklas Bo J. <nbj...@gm...> - 2012-07-09 11:09:33
|
For example: char foo(char *arr) { return arr[63]; } Should generate ldd r24, Y/Z+63, but generates: movw r31:r30, r25:r24 adiw r31:r30, 63 ld r24, Z ret On Sun, Jul 8, 2012 at 3:25 PM, Borja Ferrer <bor...@gm...> wrote: > can you give me an example where this is happening? > > 2012/7/8 Nicklas Bo Jensen <nbj...@gm...> > >> Thanks again. >> >> I've noticed that in the instruction: LDD Rd, Y/Z+q, the range for >> q generated is 0<=q<63 , whereas the instruction set also includes 63, i.e. >> 0<=q<=63. >> >> On Sat, Jul 7, 2012 at 10:33 PM, Borja Ferrer <bor...@gm...>wrote: >> >>> postinc/predec is fragile yes. Back when i was coding that part I used >>> to test it with a for loop, something like: >>> >>> for (i=0; i<x; i++) {a++ = b;} >>> being a a ptr. >>> OR >>> *a++ = 1; >>> *a++=2; >>> etc if you dont want to use a loop, better to make the test shorter. >>> >>> There are many cases where a postinc/predec should be generated but it >>> doesnt happen, so that will have to be optimized later, but for now adding >>> tests for very basic stuff is fine. >>> >>> >>> 2012/7/7 Nicklas Bo Jensen <nbj...@gm...> >>> >>>> 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 >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> > |