Menu
▾
▴
Re: [avr-llvm-devel] Regression tests
|
From: Nicklas Bo J. <nbj...@gm...> - 2012-07-09 11:09:33
|
For example:
char foo(char *arr)
{
return arr[63];
}
Should generate ldd r24, Y/Z+63, but generates:
movw r31:r30, r25:r24
adiw r31:r30, 63
ld r24, Z
ret
On Sun, Jul 8, 2012 at 3:25 PM, Borja Ferrer <bor...@gm...> wrote:
> can you give me an example where this is happening?
>
> 2012/7/8 Nicklas Bo Jensen <nbj...@gm...>
>
>> Thanks again.
>>
>> I've noticed that in the instruction: LDD Rd, Y/Z+q, the range for
>> q generated is 0<=q<63 , whereas the instruction set also includes 63, i.e.
>> 0<=q<=63.
>>
>> On Sat, Jul 7, 2012 at 10:33 PM, Borja Ferrer <bor...@gm...>wrote:
>>
>>> postinc/predec is fragile yes. Back when i was coding that part I used
>>> to test it with a for loop, something like:
>>>
>>> for (i=0; i<x; i++) {a++ = b;}
>>> being a a ptr.
>>> OR
>>> *a++ = 1;
>>> *a++=2;
>>> etc if you dont want to use a loop, better to make the test shorter.
>>>
>>> There are many cases where a postinc/predec should be generated but it
>>> doesnt happen, so that will have to be optimized later, but for now adding
>>> tests for very basic stuff is fine.
>>>
>>>
>>> 2012/7/7 Nicklas Bo Jensen <nbj...@gm...>
>>>
>>>> That sounds good for std/ldd. Regarding postinc/predec I think the
>>>> generated code is too fragile to test good, but I will look further into if
>>>> it's possible to find a better test.
>>>>
>>>> On Sat, Jul 7, 2012 at 9:59 PM, Borja Ferrer <bor...@gm...>wrote:
>>>>
>>>>> Do only one store/load per test function, that way you won't get
>>>>> predec/postinc instructions. What I was thinking of is: test x[0]=value;
>>>>> for every data type, then do the same thing for load. Then test displaced
>>>>> loads/stores with x[44]=value, you cover there std/ldd. And then finally
>>>>> postinc/predec.
>>>>>
>>>>> In fact from the piece of code you pasted, the optimal thing i guess
>>>>> would be doing st X+, reg instead of wasting reg Z, so it could be
>>>>> improved, but i'm not interested now in optimum code.
>>>>>
>>>>>
>>>>> 2012/7/7 Nicklas Bo Jensen <nbj...@gm...>
>>>>>
>>>>>> Of course, but I'm also thinking pre/post dec/inc.
>>>>>>
>>>>>> For example:
>>>>>>
>>>>>> void foo(char *arr)
>>>>>> {
>>>>>> arr[0] = 1;
>>>>>> arr[1] = 2;
>>>>>> arr[2] = 3;
>>>>>> arr[3] = 4;
>>>>>> }
>>>>>>
>>>>>> generates:
>>>>>> movw r31:r30, r25:r24
>>>>>> ldi r24, 1
>>>>>> movw r27:r26, r31:r30
>>>>>> st X+, r24
>>>>>> ldi r24, 2
>>>>>> st X, r24
>>>>>> ldi r24, 3
>>>>>> std Z+2, r24
>>>>>> ldi r24, 4
>>>>>> std Z+3, r24
>>>>>> ret
>>>>>>
>>>>>> First two stores using st and last two using std. The performance of
>>>>>> st vs std is equivalent right?
>>>>>>
>>>>>> Testing these is going to be hard if i'ts not deterministic which
>>>>>> instruction is used when.
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Sat, Jul 7, 2012 at 9:43 PM, Borja Ferrer <bor...@gm...>wrote:
>>>>>>
>>>>>>> This :
>>>>>>> void test44(int *x)
>>>>>>> {
>>>>>>> x[0]=2;
>>>>>>> }
>>>>>>> gives:
>>>>>>>
>>>>>>> movw r31:r30, r25:r24
>>>>>>> ldi r24, 2
>>>>>>> ldi r25, 0
>>>>>>> st Z, r24
>>>>>>> std Z+1, r25
>>>>>>> ret
>>>>>>>
>>>>>>>
>>>>>>> 2012/7/7 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>
>>>>>>>> Ah thanks! I cannot seem to get ld/st though?
>>>>>>>>
>>>>>>>>
>>>>>>>> On Sat, Jul 7, 2012 at 9:19 PM, Borja Ferrer <bor...@gm...
>>>>>>>> > wrote:
>>>>>>>>
>>>>>>>>> Ok great. If you want that piece of code really working, you can
>>>>>>>>> tell llvm to expand the mul nodes, you can do it in the AVRTargetLowering
>>>>>>>>> constructor with the setOperationAction.
>>>>>>>>>
>>>>>>>>> To generate those instructions it's quite easy, just use a pointer
>>>>>>>>> as an incoming argument in a function and store or load values from there,
>>>>>>>>> then use x[3] and such to get offsets in relation to a base address for
>>>>>>>>> std/ldd. It's important to test offsets in range (up to 63) and bigger ones
>>>>>>>>> which need materialization each time.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 2012/7/7 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>>>
>>>>>>>>>> Nice, that was a quick fix :) I'ts working now with -O0, with -O3
>>>>>>>>>> I'm getting the mul error.
>>>>>>>>>>
>>>>>>>>>> I mean st/std/ld/ldd.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Sat, Jul 7, 2012 at 8:51 PM, Borja Ferrer <
>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>
>>>>>>>>>>> Ok fixed, thanks for the report! If you now get the issue about
>>>>>>>>>>> the mul, it's a known thing.
>>>>>>>>>>>
>>>>>>>>>>> What do you mean by indirect mem accesses?
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> 2012/7/7 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>>>>>
>>>>>>>>>>>> Great, not just my setup then :)
>>>>>>>>>>>>
>>>>>>>>>>>> No, it's not part of any tests. I'm just trying out different
>>>>>>>>>>>> things. I'm having some trouble generating indirect memory accesses, any
>>>>>>>>>>>> tips?
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Sat, Jul 7, 2012 at 8:20 PM, Borja Ferrer <
>>>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Ok, I'm able to reproduce this crash with both clang and llc
>>>>>>>>>>>>> with -O0, with -O3 I'm getting an error because it cant select the mul
>>>>>>>>>>>>> instruction which is a known issue. I'm investigating now. Btw, is this
>>>>>>>>>>>>> part of a test you're writing?
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> 2012/7/7 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Could you try to compile the following piece of C:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> typedef unsigned char uint8_t;
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> extern uint8_t as[4];
>>>>>>>>>>>>>> extern uint8_t bs[4];
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> void foo1(void) {
>>>>>>>>>>>>>> for (uint8_t i = 0; i < 4; i++) {
>>>>>>>>>>>>>> bs[i] = as[1];
>>>>>>>>>>>>>> }
>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I'm getting some wierd errors with llc(changing a bit
>>>>>>>>>>>>>> dependent on -O0/-O3) like:
>>>>>>>>>>>>>> llvm/include/llvm/CodeGen/SelectionDAGNodes.h:534: const
>>>>>>>>>>>>>> llvm::SDValue &llvm::SDNode::getOperand(unsigned int) const: Assertion `Num
>>>>>>>>>>>>>> < NumOperands && "Invalid child # of SDNode!"' failed.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Sat, Jul 7, 2012 at 6:44 PM, Borja Ferrer <
>>>>>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> No problem, I just wanted to know what was the status, no
>>>>>>>>>>>>>>> need to rush it :)
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 2012/7/7 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> No, sorry, I've been busy lately. I will take it up again
>>>>>>>>>>>>>>>> in the next days.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Sat, Jul 7, 2012 at 6:21 PM, Borja Ferrer <
>>>>>>>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Nicklas, are you still working in those memory tests?
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> 2012/6/26 Borja Ferrer <bor...@gm...>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> What about directmem.ll to keep it a bit shorter?
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Btw, the ldd/std tests and such keep them in two separate
>>>>>>>>>>>>>>>>>> files, like load.ll and store.ll since they're going to get quite big on
>>>>>>>>>>>>>>>>>> their own.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> 2012/6/25 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I've commited the tests after going over your comments.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> The file should probably still be renamed though, we
>>>>>>>>>>>>>>>>>>> could call it loadstoredirect.ll? Then tests for st/std/ld/ldd could be in
>>>>>>>>>>>>>>>>>>> a file called loadstoreindirect.ll?
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> On Mon, Jun 25, 2012 at 1:43 PM, Borja Ferrer <
>>>>>>>>>>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Ohhh well, happy to hear it's now fixed!! Now we can
>>>>>>>>>>>>>>>>>>>> move to something else.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Memory test comments:
>>>>>>>>>>>>>>>>>>>> First of all since you're testing lds/sts i would
>>>>>>>>>>>>>>>>>>>> rename the file to something else, mem is too general, but i can't think of
>>>>>>>>>>>>>>>>>>>> a good name now, suggestions are welcome.
>>>>>>>>>>>>>>>>>>>> 1) I would remove the allocX_local tests, I'm writing
>>>>>>>>>>>>>>>>>>>> alloca tests somewhere else, and they're unrelated to sts/lds.
>>>>>>>>>>>>>>>>>>>> 2) Change all imm values to something that doesn't
>>>>>>>>>>>>>>>>>>>> contain zeros, as we mentioned previously. This will change some tests a
>>>>>>>>>>>>>>>>>>>> bit, but it's trivial to implement.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Apart of these comments, the tests look great.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Once this gets done we can then move to other memory
>>>>>>>>>>>>>>>>>>>> tests, mainly testing st/ld and std/ldd instructions. Which require testing
>>>>>>>>>>>>>>>>>>>> displacements of small and big offsets, postinc and predec operations, and
>>>>>>>>>>>>>>>>>>>> probably other stuff that I'll have to look up.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> 2012/6/25 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> OK, it's working now. I was
>>>>>>>>>>>>>>>>>>>>> missing MyFlags.Flags.setSplitPiece(j) in line 6450 in
>>>>>>>>>>>>>>>>>>>>> selectiondagbuilder.cpp. Not sure how it 'unpatched' itself.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Sorry for the trouble.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> On Mon, Jun 25, 2012 at 12:40 AM, Borja Ferrer <
>>>>>>>>>>>>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> IIRC CLI.Outs.size() should be 4 because you have 4
>>>>>>>>>>>>>>>>>>>>>> argument pieces, however ParseExternFuncCallArgs should have filled the
>>>>>>>>>>>>>>>>>>>>>> Args array to have only 1 element of size 4 because the function in reality
>>>>>>>>>>>>>>>>>>>>>> has only 1 argument. The clue here is investigating why is this failing for
>>>>>>>>>>>>>>>>>>>>>> you and not for me. I guess you're jumping to the AnalyzeArguments from
>>>>>>>>>>>>>>>>>>>>>> line 932, the one in the else clause.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> Add printf's to see what getSplitPiece returns for
>>>>>>>>>>>>>>>>>>>>>> each iteration, in theory assuming this wrong behaviour this function
>>>>>>>>>>>>>>>>>>>>>> should always return 0 so when doing Out.push_back(Size), Size is always 1.
>>>>>>>>>>>>>>>>>>>>>> The splitpiece stuff is set by
>>>>>>>>>>>>>>>>>>>>>> MyFlags.Flags.setSplitPiece(j) in line 6450 in selectiondagbuilder.cpp
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> 2012/6/24 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Yes, I have four elements with value one filled
>>>>>>>>>>>>>>>>>>>>>>> by ParseExternFuncCallArgs.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> I can't tell if ParseExternFuncCallArgs is buggy,
>>>>>>>>>>>>>>>>>>>>>>> but it seems as though it is given that there are four arguments. For
>>>>>>>>>>>>>>>>>>>>>>> example CLI.Outs.size() evaluates to 4 in line 893. Is that correctly
>>>>>>>>>>>>>>>>>>>>>>> understood?
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> On Sun, Jun 24, 2012 at 9:57 PM, Borja Ferrer <
>>>>>>>>>>>>>>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Indeed, if you mean the Size var set in line 750 it
>>>>>>>>>>>>>>>>>>>>>>>> should be 4. The Args vector should only contain 1 element because you only
>>>>>>>>>>>>>>>>>>>>>>>> have 1 argument in the function and the size should be set to 4. This is
>>>>>>>>>>>>>>>>>>>>>>>> the reason of why the arguments aren't reversed. In your case I guess you
>>>>>>>>>>>>>>>>>>>>>>>> have 4 elements of size 1. This vector is filled by
>>>>>>>>>>>>>>>>>>>>>>>> ParseExternFuncCallArgs() or by ParseFunctionArgs() depending on the call
>>>>>>>>>>>>>>>>>>>>>>>> type, so the problem is there, in this case ParseExternFuncCallArgs is
>>>>>>>>>>>>>>>>>>>>>>>> doing the work right? So this is the function you should debug.
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> 2012/6/24 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> You're right, it's the -O3 flag.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> I'm stepping through the code with the debugger
>>>>>>>>>>>>>>>>>>>>>>>>> right now. The indirect call does trigger AnalyzeArguments to reach line
>>>>>>>>>>>>>>>>>>>>>>>>> 768 (std::reverse).
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> There is one thing puzzling me a bit, the argument
>>>>>>>>>>>>>>>>>>>>>>>>> size is calculated to be 1, i.e. Args[0] is 1, but shouldn't a 64bit
>>>>>>>>>>>>>>>>>>>>>>>>> argument evaluate to 4 in size?
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> On Sun, Jun 24, 2012 at 8:16 PM, Borja Ferrer <
>>>>>>>>>>>>>>>>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> With that command line I get the same code as you
>>>>>>>>>>>>>>>>>>>>>>>>>> (now i get the spill i wasn't getting before), however i still get correct
>>>>>>>>>>>>>>>>>>>>>>>>>> code:
>>>>>>>>>>>>>>>>>>>>>>>>>> movw r31:r30, r25:r24
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> std Y+1, r30
>>>>>>>>>>>>>>>>>>>>>>>>>> std Y+2, r31
>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r18, 136
>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r19, 153
>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r20, 255
>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r21, 238
>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r22, 221
>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r23, 204
>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r24, 187
>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r25, 170
>>>>>>>>>>>>>>>>>>>>>>>>>> icall
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> This is my usual command line:
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> clang -O3 -emit-llvm test.c -S -o test.bc
>>>>>>>>>>>>>>>>>>>>>>>>>> -ccc-host-triple avr-generic-generic -ccc-clang-archs avr
>>>>>>>>>>>>>>>>>>>>>>>>>> llc -O3 -march=avr test.bc -pre-RA-sched=default
>>>>>>>>>>>>>>>>>>>>>>>>>> -regalloc=avrgreedy -debug-only=isel -o test.S -print-isel-input
>>>>>>>>>>>>>>>>>>>>>>>>>> -print-after-all -stats 2> dump
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> 2012/6/24 Nicklas Bo Jensen <nbj...@gm...>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> All patches should be installed and it has
>>>>>>>>>>>>>>>>>>>>>>>>>>> worked earlier when I wrote that test. I also have the clang patches, but I
>>>>>>>>>>>>>>>>>>>>>>>>>>> don't think it has anything to do with those.
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> To compile icall.c, I would for example do:
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> ../build/Debug+Asserts/bin/clang icall.c -S
>>>>>>>>>>>>>>>>>>>>>>>>>>> -emit-llvm -o icall.ll
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> to view the llvm ir and to get assembler:
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> ../build/Debug+Asserts/bin/llc icall.ll
>>>>>>>>>>>>>>>>>>>>>>>>>>> -march=avr -O3 -o icall.s
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> I have that exact code as you describe, I will
>>>>>>>>>>>>>>>>>>>>>>>>>>> printf debug it later tonight.
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> On Sun, Jun 24, 2012 at 1:58 PM, Borja Ferrer <
>>>>>>>>>>>>>>>>>>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Ok the problem I see is that you're not getting
>>>>>>>>>>>>>>>>>>>>>>>>>>>> the arguments pieces reversed. Can you check the code that performs this?
>>>>>>>>>>>>>>>>>>>>>>>>>>>> This is done inside AnalyzeArguments, exactly in line 768 done by
>>>>>>>>>>>>>>>>>>>>>>>>>>>> std::reverse. Also check that this function is being called in line 932.
>>>>>>>>>>>>>>>>>>>>>>>>>>>> That should give you enough info to debug it, use fprintfs to print out
>>>>>>>>>>>>>>>>>>>>>>>>>>>> debugging info to help you otherwise it's going to be hard to debug.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> I'm assuming you have all patches installed,
>>>>>>>>>>>>>>>>>>>>>>>>>>>> including clang patches right?
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Paste the command line you're using to call
>>>>>>>>>>>>>>>>>>>>>>>>>>>> clang because I'm not getting
>>>>>>>>>>>>>>>>>>>>>>>>>>>> std Y+1, r30
>>>>>>>>>>>>>>>>>>>>>>>>>>>> std Y+2, r31
>>>>>>>>>>>>>>>>>>>>>>>>>>>> or any stack allocation as you get.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2012/6/24 Nicklas Bo Jensen <nbj...@gm...
>>>>>>>>>>>>>>>>>>>>>>>>>>>> >
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Still gives the wrong code:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r24, 136
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r25, 153
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r22, 255
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r23, 238
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r20, 221
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r21, 204
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r18, 187
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r19, 170
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I have also attached the output.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Sun, Jun 24, 2012 at 1:38 AM, Borja Ferrer
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> <bor...@gm...> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> What do you get with the following code?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> long icall(long (*x)(long long))
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> {
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> return x(0xAABBCCDDEEFF9988LL)+5;
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> John could you check if you're getting the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> right code?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2012/6/24 Nicklas Bo Jensen <
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> nbj...@gm...>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> No, regular calls/and/xor/or works fine
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> with 3335977107.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Sun, Jun 24, 2012 at 12:45 AM, Borja
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Ferrer <bor...@gm...> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> This is very weird. Do you get this problem
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> with other code when you use this particular number (3335977107) like with
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> a regular call or with an and/xor/or?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2012/6/24 Nicklas Bo Jensen <
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> nbj...@gm...>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I also get the wrong result compiling that
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> snippet...
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I have tried to revert the last patches,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> those that not needed for syncing with the llvm repo, same result. I have
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> also updated llvm. Same result.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Please let me know if you have any ideas
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> what it could be :)
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Sat, Jun 23, 2012 at 8:15 PM, Borja
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Ferrer <bor...@gm...> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for the patch there!
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Very interesting because I'm not getting
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> that error in icall at all, I'm getting the second code snippet you pasted.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> With this code:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> long icall(long (*x)(long))
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> {
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> return x(0xC6D6F893)+5;
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I get:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> movw r19:r18, r25:r24
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r22, 147
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r23, 248
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r24, 214
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r25, 198
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> movw r31:r30, r19:r18
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> icall
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> subi r22, 251
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> sbci r23, 255
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> sbci r24, 255
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> sbci r25, 255
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ret
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Can you re check to see what's going on?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> The comments in zext.ll and sext.ll are
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> there for reference, they cover the cases implemented in the pseudo
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> instruction expander, that way we're able to know what test covers each
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> case.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Yes i have comments for the memory tests,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I'll write them in the next email. We should continue with memory
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> operations, there are many more things to test there that i'll list next
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> time. For now we have to see what's going on with that failing test.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2012/6/23 Nicklas Bo Jensen <
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> nbj...@gm...>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I've committed the small change in
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Targets.cpp as you described.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> We are breaking a test now. In call.s,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> icall is loading arguments incorrectly now.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> The instruction %1 = call i32 %foo(i32
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 3335977107)
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> gives:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r24, 147
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r25, 248
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r22, 214
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r23, 198
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> but should give:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r22, 147
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r23, 248
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r24, 214
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ldi r25, 198
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Also in your new sext.ll and zext.ll
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> tests there are some comments above each function, should these be deleted?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Last did you have any comments above the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> memory tests?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Any preferences for what I should
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> continue on?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Tue, Jun 19, 2012 at 1:47 PM, Borja
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Ferrer <bor...@gm...> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Nicklas, in the meantime while the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> patch gets fixed you can manually fix the error by adding that virtual
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> BuiltinVaListKind getBuiltinVaListKind() const function in the AVR class,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> in the above link you have an example on how to do it.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2012/6/19 Borja Ferrer <
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> bor...@gm...>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> John can you fix this issue, i have a
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> different local copy of Targets.cpp that i don't want to change now with
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the patch in SVN. The fix is in this commit
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?r1=158334&r2=158592&sortby=date&diff_format=h
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> so updating the patch file will do it.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2012/6/18 Borja Ferrer <
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> bor...@gm...>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> You've done it right, the problem is
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> here:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?r1=158334&r2=158592&sortby=date&diff_format=h
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> it's caused by a recent API change
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> that has broken out of tree targets, i'll fix it soon.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2012/6/18 Nicklas Bo Jensen <
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> nbj...@gm...>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Yes of course.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I'm having some problems compiling
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> getting the following error:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> /avr-llvm/llvm/tools/clang/lib/Basic/Targets.cpp:4172:16:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> error: allocating an object of
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> abstract class type
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 'AVRTargetInfo'
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> return new AVRTargetInfo(T);
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ^
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> /avr-llvm/llvm/tools/clang/lib/Basic/../../include/clang/Basic/TargetInfo.h:406:29:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> note:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> unimplemented pure virtual
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> method 'getBuiltinVaListKind' in 'AVRTargetInfo'
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> virtual BuiltinVaListKind
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> getBuiltinVaListKind() const = 0;
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ^
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 1 error generated.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I think I have applied the patches
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> from svn correctly, at least did it a few times. I'm not stuck, but any
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ideas would be nice.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Mon, Jun 18, 2012 at 1:46 PM,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Borja Ferrer <bor...@gm...>wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Ok great!
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> One more thing about the icall
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> test, it can be further reduced because:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> %1 = alloca i32 (i32)*
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> store i32 (i32)* %foo, i32
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> (i32)** %1
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> %2 = load i32 (i32)** %1
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> all that code is redundant, you're
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> getting it because you've ran it with -O0. If you think about it, you've
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> allocated an i32 var in the stack just to copy the value of foo but then
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> it's not used at all. This code is typical from unoptimized builds, so you
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> can reduce it to something like:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> %3 = call i32 %foo(i32 3335977107)
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> %4 = add nsw i32 %3, 5
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> here you're using the foo incoming
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> arg directly.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> So as a recommendation, I would
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> also run clang with -03 to get optimized outputs that are usually clean of
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> redundant allocas.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2012/6/18 Nicklas Bo Jensen <
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> nbj...@gm...>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I've committed the updated tests,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> excluding mem.ll awaiting your feedback on it (Besides not using immediate
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 0).
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Mon, Jun 18, 2012 at 1:10 AM,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Borja Ferrer <
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> return test:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 1) you can remove the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> @return8_reg() test since it's completely commented out.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2) in return64_arg2 you're always
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> going to push r29:r28 as we disccused in a previous email, so you can
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> remove the SPLOW/HI vars, also SP is going to be always the Y reg pair.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Using Z here would be a bug.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 3) in return64_trunc same as in
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> point 2.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 4) add a :TODO: test returning
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> byval structs.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> call test:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 1) in the icall test, the code
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> you're testing is a bit fragile. Basically if you run it with -O3 it will
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> get optimized away and you wont get an icall. Instead, generate the icall
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> using a function pointer from an incoming argument.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2) do the same as in 1 but
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> passing an argument to the icall (any data type will do). (remember i had
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> to fix this last week, so testing it is imporant in case it breaks again in
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the future)
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 3) do the same as in 1 using the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> return value of the icall (any data type will do).
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> One general thing concerning all
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> written tests up to now, can you change all imm values (including the ones
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> already commited), so that you don't test things like "ldi reg, 0". So for
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> a 16bit type instead of using a 5 use a value were none of the registers
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> are zero like 0xABCD instead of 0x0005 where the hi part is 0. In the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> future we'll add an optimization that relies on r0 having a 0, so many
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> tests will break because of this since we could get a mov reg, r0 instead
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> of an ldi.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I'll reply tomorrow with comments
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> about the memory tests.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2012/6/17 Borja Ferrer <
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> bor...@gm...>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks, I'm reviewing them now
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> El viernes, 15 de junio de 2012,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Nicklas Bo Jensen escribió:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Please review these tests.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Any comments?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Jun 6, 2012 at 2:46 AM,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Borja Ferrer <
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Fixed in SVN, let me know any
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> other issues. Thanks for reporting it!
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2012/6/5 Borja Ferrer <
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> bor...@gm...>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Something has changed in llvm
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> or maybe it never worked, can't remember, but I'm unable to pass arguments
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> through an icall, I'm investigating for a fix. In the meantime you can
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> disable that assertion until I come with a proper fix.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2012/6/4 Borja Ferrer <
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> bor...@gm...>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I'll take a look to it shortly.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2012/6/3 Nicklas Bo Jensen <
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> nbj...@gm...>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Ok. Sure your explanations are
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> really clear :) Thanks!
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I'm having some trouble testing
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> icall, which i'm doing using a function pointer. I've attached a small C
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> example, the corresponding cleaned up llvm code and the output of running
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> llc on it. There is a failed assertion.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Any thoughts?On Fri, Jun 1,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2012 at 7:04 PM, Borja Ferrer <
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> bor...@gm...> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Just checking for the sub instructions that allocate and restore the stack
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> should be enough, also check that the allocated size is correct, the rest
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> of stuff can be omitted. I would also check the whole sequence including
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the interrupt instructions in a single test, no need to do it everywhere.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> When we get to test allocas we'll have to test for prologue and epilogue
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> code, which is very similar to the one you're seeing here. About testing
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> push and pops: no need to test that, it's part of the prologue/epilogue of
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the function, so it's not related to calls, also, that is handled by llvm
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> so I wouldn't test that at all. Remember to test the icall instruction, as
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> far as I remember it needs to be tweaked because for now it doesn't save
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the registers between calls like regular calls do. Ok, memory instructions,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> most of the instructions you see in that file that end up with a "w" are
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> pseudo instructions (excluding sbiw/adiw, movw or other real ones that work
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> with 16bit data) meaning that they dont get into the final assembly since
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> they don't exist in the instruction set. It's a trick to make llvm think
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> they're supported instructions that way, movws and the real 16bit insts get
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> generated, however these pseudo ops get expanded into 8bit ops, that's why
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> you wont never see those in the final assembly, and if you ever see one
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> then it's a bug. So if you do an and with 16bit types, llvm selects the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> andw pseudo op, and a later pass expands it into two real 8bit and insts
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> that are supported by the avr inst set, dont' know if this makes things
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> clear. 2012/5/31 Nicklas Bo Jensen <
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> nbj...@gm...> Checking
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> for stack allocation and restore could be something like:; CHECK: in
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> [[REG1:r[0-9]+]], 61; CHECK: in [[REG2:r[0-9]+]], 62; CHECK: sbiw
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> [[REG2]]:[[REG1]], 4 ; CHECK: out 62, [[REG2]]; CHECK: out 61, [[REG1]];
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ... other tests; CHECK: in [[REG1:r[0-9]+]], 61;
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> CHECK: in [[REG2:r[0-9]+]], 62; CHECK: subi [[REG1]], 254 ; CHECK:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> sbci [[REG2]], 255; CHECK: out 62, [[REG2]]; CHECK: out 61, [[REG1]]Is it
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> something like that you have in mind? Should disabling and enabling
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> interupts (cli+writeback of SREG) also be tested? Checkin
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
|
