From: Stepan D. <stp...@na...> - 2013-07-09 13:05:08
|
Hello Borja, Sorry for latency, I had to be on dev. meeting. I've implemented optimization for memory constraint support. The patch is in attachment. P.S.: When I started to work on this opt, I forgot about multibyte constraints patch is not committed. So I can send it after we commit this opt. -Stepan. Borja Ferrer wrote: > Yes but the first goal now is to have inline asm feature complete xD > Then we can move into other places of the library. > > Taking a look at the previous emails this is what needs still to be done: > 1) Implement the missed optimization above for the memory constraint. > Also see if you can come with further cases. Remember to add a test case. > 2) Implement the stuff in the "C names used in assembler code" section > of the avrlibc manual. You mentioned some issues here in a previous email. > 3) Implement the a0, a1, etc... constraints. > > > > 2013/7/1 Stepan Dyatkovskiy <stp...@na... <mailto:stp...@na...>> > > Hi Borja. > 1. OK. Will add it. > 2. > > > movw r30, r24 > > adiw r30, 1 > Yup, we can replace it with "load r24, Z+1". I'll improve > LowerINLINEASM a bit then. > > 3. If I'm got right, our main goal now is to get avr-libc > compilable? Whould you tell me would else remained to do with > inline-asm? > > -Stepan. > > Borja Ferrer wrote: > > 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... > <mailto:bor...@gm...> > <mailto:bor...@gm... <mailto: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... > <mailto:bor...@gm...> > <mailto:bor...@gm... > <mailto: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... > <mailto:bor...@gm...> > <mailto:bor...@gm... > <mailto: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... > <mailto:bor...@gm...> > <mailto:bor...@gm... > <mailto: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... > <mailto:bor...@gm...> > <mailto:bor...@gm... > <mailto: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... <mailto:stp...@na...> > <mailto:stp...@na... > <mailto: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...> > <mailto:bor...@gm... > <mailto:bor...@gm...>> > > <mailto:bor...@gm... <mailto:bor...@gm...> > <mailto: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...> <mailto:stp...@na... > <mailto:stp...@na...>> > > <mailto:stp...@na... <mailto:stp...@na...> > > <mailto: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...>> > > <mailto:stp...@na... > <mailto:stp...@na...> > <mailto:stp...@na... > <mailto:stp...@na...>>> > <mailto:stp...@na... > <mailto:stp...@na...> > <mailto:stp...@na... > <mailto:stp...@na...>> > > <mailto: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...>>__> > > <mailto:bor...@gm... > <mailto:bor...@gm...> > <mailto:bor...@gm... > <mailto:bor...@gm...>> > > <mailto:bor...@gm... > <mailto:bor...@gm...> > <mailto:bor...@gm... > <mailto:bor...@gm...>>__>__> > > <mailto:bor...@gm... > <mailto:bor...@gm...> > <mailto:bor...@gm... > <mailto:bor...@gm...>> > > <mailto:bor...@gm... > <mailto:bor...@gm...> > <mailto:bor...@gm... > <mailto:bor...@gm...>>__> > > <mailto: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...>>> > > <mailto:stp...@na... > <mailto:stp...@na...> > <mailto:stp...@na... > <mailto:stp...@na...>> > <mailto:stp...@na... > <mailto:stp...@na...> > <mailto:stp...@na... > <mailto:stp...@na...>>>> > > <mailto:stp...@na... > <mailto:stp...@na...> > <mailto:stp...@na... > <mailto:stp...@na...>> > > <mailto:stp...@na... > <mailto:stp...@na...> > <mailto:stp...@na... > <mailto:stp...@na...>>> > <mailto: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...>>> > > <mailto:stp...@na... > <mailto:stp...@na...> > <mailto:stp...@na... > <mailto:stp...@na...>> > <mailto:stp...@na... > <mailto:stp...@na...> > <mailto:stp...@na... > <mailto:stp...@na...>>>> > > <mailto:stp...@na... > <mailto:stp...@na...> > <mailto:stp...@na... > <mailto:stp...@na...>> > > <mailto:stp...@na... > <mailto:stp...@na...> > <mailto:stp...@na... > <mailto:stp...@na...>>> > <mailto: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...>>__> > > <mailto:bor...@gm... > <mailto:bor...@gm...> > <mailto:bor...@gm... > <mailto:bor...@gm...>> > > <mailto:bor...@gm... > <mailto:bor...@gm...> > <mailto:bor...@gm... > <mailto:bor...@gm...>>__>__> > > <mailto:bor...@gm... > <mailto:bor...@gm...> > <mailto:bor...@gm... > <mailto:bor...@gm...>> > > <mailto:bor...@gm... > <mailto:bor...@gm...> > <mailto:bor...@gm... > <mailto:bor...@gm...>>__> > > <mailto: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__forge.net> > > <http://source <http://sourceforge.net> > > > > > > > > > > |