Menu
▾
▴
Re: [avr-llvm-devel] Inline assembly. Mostly just a stub.
|
From: Borja F. <bor...@gm...> - 2013-07-09 14:22:08
|
Jej no problem, I know you've been busy with some ARM stuff.
Yes let's get this in first, and then we can talk about the multibyte stuff.
About the patch:
Is all the code in AVRISelLowering.cpp necesarry? I find it to be a very
"manual" way of doing this optimization, it feels like not being very
general, so please confirm me that this the only way of doing it. Im asking
this in case in AVRISelDAGToDAG.cpp you could add some code to match this
sort of DAGs with the existing adressing mode matching functions to avoid
the code in the other file. Maybe look at what other backends do?
In AVRISelDAGToDAG.cpp you've removed this line:
cast<MemSDNode>(Op)->getMemoryVT().getSimpleVT();
to be:
+ if (MemSDNode* MemNode = dyn_cast<MemSDNode>(Op))
Is this condition always going to be true? the cast above never failed for
me so in theory the dyn_cast should always be true, making the rest of
elses there always false.
2013/7/9 Stepan Dyatkovskiy <stp...@na...>
> 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____forge.net
>> >
>>
>> <http://source__forge.net>
>>
>> <http://source <
>> http://sourceforge.net>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>
|
