|
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>
>
>
>
>
>
>
>
>
>
>
|