Menu
▾
▴
Re: [avr-llvm-devel] Regression tests
|
From: Borja F. <bor...@gm...> - 2012-07-08 13:26:03
|
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
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
|
