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