From: Borja F. <bor...@gm...> - 2013-06-30 12:42:13
|
1) Yes, an assert could do it. Fixup your patch to only allow A down to D constraints. 2) This code was compiled with -O3, cant you reproduce it? 2013/6/29 Borja Ferrer <bor...@gm...> > Ok more things, I've looked into the 64bit constraints and gcc doesnt seem > to support them, for this code: > > unsigned long long delay(unsigned long long a, unsigned long long b) > > { > uint8_t cnt; > asm volatile ( > "add %A0, %A1" "\n\t" > "add %B0, %B1" "\n\t" > "add %C0, %C1" "\n\t" > "add %D0, %D1" "\n\t" > "add %E0, %E1" "\n\t" > "add %F0, %F1" "\n\t" > "add %G0, %G1" "\n\t" > "add %H0, %H1" "\n\t" > : "=r" (a) > : "r" (b)); > return a; > } > > gcc produces (ignoring frames): > movw r18,r10 > movw r20,r12 > movw r22,r14 > movw r24,r16 > /* #APP */ > ; 266 "test.c" 1 > add r10, r18 > add r11, r19 > add r12, r20 > add r13, r21 > add r10, r18 > add r10, r18 > add r10, r18 > add r10, r18 > > ; 0 "" 2 > /* #NOAPP */ > movw r18,r10 > movw r20,r12 > movw r22,r14 > movw r24,r16 > > notice how the last 4 add instructions are wrong. > > > > 2013/6/29 Borja Ferrer <bor...@gm...> > >> Ignore my previous email, it was my fault. Now, for the following code: >> >> uint8_t delay(unsigned char *p) >> >> { >> uint8_t cnt=8; >> asm volatile ( >> "ld %0, %1" "\n\t" >> : "=r" (cnt) >> : "Q" (p[1])); >> return cnt; >> } >> >> we get: >> movw r30, r24 >> adiw r30, 1 >> ;APP >> ld r24, Z >> >> ;NO_APP >> ret >> >> Ideally, that adiw should be folded into the load. >> >> >> >> 2013/6/29 Borja Ferrer <bor...@gm...> >> >>> Stepan I've been testing the memory constraint but im getting an >>> assertion for the following code: >>> >>> uint8_t delay(unsigned char **p) >>> { >>> uint8_t cnt=8; >>> asm volatile ( >>> "ld %0, %1" "\n\t" >>> : "=r" (cnt) >>> : "Q" (p[7])); >>> return cnt; >>> } >>> >>> What's wrong in here? >>> >>> >>> 2013/6/29 Borja Ferrer <bor...@gm...> >>> >>>> 1) Ok then, I will look into it. >>>> 2) Yes, in theory types should be legalized up to i16 max, but for some >>>> reason i got that assertion triggered, i think it was with multibyte inline >>>> asm, so this has to be solved. >>>> >>>> >>>> 2013/6/28 Borja Ferrer <bor...@gm...> >>>> >>>>> Hello Stepan, I will take a look to your patch later, but some >>>>> questions: >>>>> >>>>> 1) does avr-gcc support 64bit values? >>>>> 2) now that you're working with big data types, does it make sense to >>>>> remove the assert I commented out about value types that are different to >>>>> i8 AND i16? >>>>> >>>>> >>>>> >>>>> 2013/6/28 Stepan Dyatkovskiy <stp...@na...> >>>>> >>>>>> Hi Borja. >>>>>> This is almost the final multibyte constraint patch. Tests are not >>>>>> created yet. >>>>>> >>>>>> Currently I have issue with 64 bit values. Even if I pass them as >>>>>> inline asm operands, llvm still truncates them to i32. >>>>>> >>>>>> What works: >>>>>> >>>>>> long a; // 32 bit >>>>>> void f() { >>>>>> asm("instr %A0 %B0 %C0 %D0": : "r"(a)); >>>>>> } >>>>>> >>>>>> >>>>>> What doesn't work: >>>>>> >>>>>> long long a; // 64 bit >>>>>> void f() { >>>>>> asm("instr %A0 %B0 %E0": : "r"(a)); >>>>>> } >>>>>> >>>>>> The last one fires assertion since llvm allocates registers for the >>>>>> first 32 bits only. >>>>>> >>>>>> -Stepan. >>>>>> >>>>>> Borja Ferrer wrote: >>>>>> >>>>>>> Yes fine >>>>>>> >>>>>>> El jueves, 27 de junio de 2013, Stepan Dyatkovskiy escribió: >>>>>>> >>>>>>> OK. Will do. >>>>>>> Currently there is a patch with memory constraint and draft >>>>>>> multibyte constraint (supports only A and B). To be clean under >>>>>>> multibyte constraints I mean something like this: >>>>>>> asm volatile("mov __tmp_reg__, %A0" "\n\t" >>>>>>> "mov %A0, %B0" "\n\t" >>>>>>> "mov %B0, __tmp_reg__" "\n\t" >>>>>>> : "=r" (value) >>>>>>> : "0" (value) >>>>>>> ); >>>>>>> >>>>>>> -Stepan. >>>>>>> Borja Ferrer wrote: >>>>>>> >>>>>>> btw, if this is now feature complete you can commit it. >>>>>>> >>>>>>> >>>>>>> 2013/6/27 Borja Ferrer <bor...@gm... >>>>>>> <mailto:bor...@gm...>**> >>>>>>> >>>>>>> No, that code is a personal modification of your >>>>>>> examples. >>>>>>> If you >>>>>>> aren't getting the correct output from clang that's >>>>>>> because you >>>>>>> probably didn't apply a patch i commited last week for >>>>>>> clang in our SVN. >>>>>>> The assertion you're getting looks reasonable, it's an >>>>>>> old >>>>>>> friend of >>>>>>> mine. The reason is that if Y is being reserved as the >>>>>>> frame >>>>>>> pointer, you cant have an instruction that uses 2 >>>>>>> registers >>>>>>> when >>>>>>> only Z is available. I wouldn't mind too much about it. >>>>>>> >>>>>>> Now, my 2nd question: >>>>>>> >>>>>>> Is there anything else that needs to be covered for the >>>>>>> memory >>>>>>> constraint? >>>>>>> >>>>>>> >>>>>>> 2013/6/27 Stepan Dyatkovskiy <stp...@na... >>>>>>> <mailto:stp...@na...>> >>>>>>> >>>>>>> This one looks good. Though ./llc emits "run out of >>>>>>> registers >>>>>>> allocation. I'll look at it. Is that from avr-libc? >>>>>>> -Stepan. >>>>>>> Borja Ferrer wrote: >>>>>>> >>>>>>> Yes, and then I replied the following: >>>>>>> >>>>>>> For this C code: >>>>>>> >>>>>>> char delay_count; >>>>>>> char aaa; >>>>>>> uint8_t delay(unsigned char p) >>>>>>> { >>>>>>> uint8_t cnt; >>>>>>> asm volatile ( >>>>>>> "inst %0, %1" "\n\t" >>>>>>> : "=Q" (delay_count) >>>>>>> : "Q" (aaa)); >>>>>>> return cnt; >>>>>>> } >>>>>>> >>>>>>> Clang produces: >>>>>>> >>>>>>> define i8 @delay(i8 %p) #0 { >>>>>>> entry: >>>>>>> tail call void asm sideeffect "inst $0, >>>>>>> $1\0A\09", >>>>>>> "=*Q,*Q"(i8* >>>>>>> @delay_count, i8* @aaa) #2, !srcloc !4 >>>>>>> ret i8 undef >>>>>>> } >>>>>>> >>>>>>> notice the *Q constraints, so is there anything >>>>>>> wrong in there? >>>>>>> >>>>>>> Is there anything else that needs to be covered >>>>>>> for the >>>>>>> memory constraint? >>>>>>> >>>>>>> >>>>>>> 2013/6/27 Stepan Dyatkovskiy <stp...@na... >>>>>>> <mailto:stp...@na...> <mailto: >>>>>>> stp...@na... >>>>>>> <mailto:stp...@na...>>> >>>>>>> >>>>>>> >>>>>>> Hi Borja, >>>>>>> >>>>>>> > Stepan, reply in this thread to my 2 >>>>>>> questions again >>>>>>> if the email >>>>>>> got lost. >>>>>>> >>>>>>> My prev. reply: >>>>>>> >>>>>>> >>>>>>> > Ahh, I didn't know avr-gcc was buggy on >>>>>>> this >>>>>>> feature, that's probably >>>>>>> > the reason for why the code i pasted in >>>>>>> my >>>>>>> previous >>>>>>> email didn't >>>>>>> work. >>>>>>> ... >>>>>>> > What's wrong with clang? I fixed clang >>>>>>> inline asm >>>>>>> support back on >>>>>>> > friday, so is there anything else that >>>>>>> is >>>>>>> broken? >>>>>>> It should emit '*Q' instead of 'Q'. That's >>>>>>> why >>>>>>> code >>>>>>> from prev. email >>>>>>> didn't work, just compare -emit-llvm -S >>>>>>> output of >>>>>>> avr-clang with >>>>>>> other backends, >>>>>>> >>>>>>> > About the getPointerRegClass function, >>>>>>> what >>>>>>> is it >>>>>>> used for? Also, >>>>>>> please >>>>>>> > move the implementation to the cpp file >>>>>>> instead of >>>>>>> leaving it in >>>>>>> the .h. >>>>>>> Currently, it is used in registers >>>>>>> coelescing >>>>>>> pass. >>>>>>> While being >>>>>>> registers inflating, llvm lookups all >>>>>>> register >>>>>>> uses. If >>>>>>> it found >>>>>>> that register is used by inline-asm as >>>>>>> memory >>>>>>> operand, >>>>>>> it requests >>>>>>> the pointer class with this method (I think >>>>>>> the largest >>>>>>> one). But >>>>>>> may be in future this method will be used >>>>>>> in >>>>>>> more cases. >>>>>>> >>>>>>> avr-gcc has buggy implementation of memory >>>>>>> constraint, >>>>>>> that's why >>>>>>> avr-libs doesn't use it at all. We have a >>>>>>> chance to be >>>>>>> first here >>>>>>> >>>>>>> -Stepan >>>>>>> >>>>>>> Borja Ferrer wrote: >>>>>>> >>>>>>> Stepan, reply in this thread to my 2 >>>>>>> questions >>>>>>> again if the >>>>>>> email got lost. >>>>>>> >>>>>>> >>>>>>> 2013/6/27 Borja Ferrer < >>>>>>> bor...@gm... >>>>>>> <mailto:bor...@gm...> >>>>>>> <mailto:bor...@gm... >>>>>>> <mailto:bor...@gm...>**> >>>>>>> <mailto:bor...@gm... >>>>>>> <mailto:bor...@gm...> >>>>>>> <mailto:bor...@gm... >>>>>>> <mailto:bor...@gm...>**>__>__> >>>>>>> >>>>>>> >>>>>>> No, your last email only says : >>>>>>> "So, >>>>>>> can I >>>>>>> commit that memory >>>>>>> constraint patch?" >>>>>>> >>>>>>> >>>>>>> 2013/6/27 Stepan Dyatkovskiy >>>>>>> <stp...@na... <mailto:stp...@na...> >>>>>>> <mailto:stp...@na... >>>>>>> <mailto:stp...@na...>> >>>>>>> <mailto:stp...@na... >>>>>>> <mailto:stp...@na...> <mailto: >>>>>>> stp...@na... >>>>>>> <mailto:stp...@na...>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> Hm... Didn't you get my >>>>>>> previous >>>>>>> mail? If >>>>>>> not, I think, >>>>>>> I have >>>>>>> to change my mail box. >>>>>>> >>>>>>> -Stepan >>>>>>> >>>>>>> >>>>>>> -------- Original Message >>>>>>> -------- >>>>>>> Subject: Re: [avr-llvm-devel] >>>>>>> Inline >>>>>>> assembly. Mostly >>>>>>> just a stub. >>>>>>> Date: Tue, 25 Jun 2013 >>>>>>> 18:30:46 +0400 >>>>>>> From: Stepan Dyatkovskiy >>>>>>> <stp...@na... <mailto:stp...@na...> >>>>>>> <mailto:stp...@na... >>>>>>> <mailto:stp...@na...>> >>>>>>> <mailto:stp...@na... >>>>>>> <mailto:stp...@na...> <mailto: >>>>>>> stp...@na... >>>>>>> <mailto:stp...@na...>>>> >>>>>>> To: Borja Ferrer >>>>>>> <bor...@gm... >>>>>>> <mailto:bor...@gm...> >>>>>>> <mailto:bor...@gm... >>>>>>> <mailto:bor...@gm...>**> >>>>>>> <mailto:bor...@gm... >>>>>>> <mailto:bor...@gm...> >>>>>>> <mailto:bor...@gm... >>>>>>> <mailto:bor...@gm...>**>__>__> >>>>>>> CC: avr-llvm >>>>>>> <avr-llvm-devel@lists.__source**______forge.net<http://source______forge.net> >>>>>>> <http://source____forge.net> >>>>>>> <http://source__forge.net> >>>>>>> <http://source <http://sourceforge.net >>>>>>> > >>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> > |