From: Stepan D. <stp...@na...> - 2013-06-29 21:35:01
|
Yup. We can keep that behavour then. May be we have to add an assertion, than only up to 32 bit types are supported. About movw + adiw, have you tried it with O2? > 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 > >> 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 >> >>> 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 >>> >>>> 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 >>>> >>>>> 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 >>>>> >>>>>> 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 >>>>>> >>>>>>> > >>>>>>> >>>>>>> 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 >>>>>> >>>>>>> > >>>>>>> >>>>>>> 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 >>>>>> >>>>>>> >>>>>> >>>>>>> >> >>>>>>> >>>>>>> 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 >>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>>>> > >>>>>>> >>>>>>> >>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>>>> >__>__> >>>>>>> >>>>>>> No, your last email only says : "So, >>>>>>> >>>>>>> can I >>>>>>> >>>>>>> commit that memory >>>>>>> >>>>>>> constraint patch?" >>>>>>> >>>>>>> 2013/6/27 Stepan Dyatkovskiy >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>>>> > >>>>>>> >>>>>>> >>>>>> >>>>>>> >>>>>> >>>>>>> >>> >>>>>>> >>>>>>> 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 >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>>>> > >>>>>>> >>>>>>> >>>>>> >>>>>>> >>>>>> >>>>>>> >>> >>>>>>> >>>>>>> To: Borja Ferrer >>>>>>> >>>>>>> >>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>>>> > >>>>>>> >>>>>>> >>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>>>> >__>__> >>>>>>> >>>>>>> CC: avr-llvm >>>>>>> >>>>>>> >>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- Truly yours, Stepan Dyatkovskiy |