From: Nicklas Bo J. <nbj...@gm...> - 2012-07-08 08:43:33
|
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 >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> > |