Menu
▾
▴
Re: [avr-llvm-devel] Regression tests
|
From: Nicklas Bo J. <nbj...@gm...> - 2012-07-07 20:18:25
|
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
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
|
