Menu
▾
▴
avr-llvm-devel
|
From: Borja F. <bor...@gm...> - 2013-06-29 15:13:10
|
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>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
|
|
From: Stepan D. <stp...@na...> - 2013-06-27 17:21:21
|
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...>>
>
> No, your last email only says : "So, can I commit that memory
> constraint patch?"
>
>
> 2013/6/27 Stepan Dyatkovskiy <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...>>
> To: Borja Ferrer <bor...@gm...
> <mailto:bor...@gm...>>
> CC: avr-llvm <avr-llvm-devel@lists.__sourceforge.net
> <mailto:avr...@li...>>, John Myers
> <ato...@gm... <mailto:ato...@gm...>>
>
> Hi Borja,
>
> 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
>
>
>
>
> 2013/6/25 Stepan Dyatkovskiy <stp...@na...
> <mailto:stp...@na...> <mailto:stp...@na...
> <mailto:stp...@na...>>>
>
> Hi Borja.
> This is new patch. Seems I've moved memory constraint
> implementation
> to the same level like all other constraints. Note
> avr-gcc has buggy
> implementation of memory constraint, that's why
> *avr-libc doesn't
> use it*.
>
> I removed restriction for Y,Z in
> getLargestLegalSuperClass, so these
> registers could be inflated now. But there was
> unimplemented
> getPointerRegClass method in AVRRegistersInfo. So
> currently I have
> restricted it to Y and Z. Though suppose we can extend
> it in future
> to XYZ set.
> Look changes in inline-asm.ll to see what exactly is
> supported now.
>
> About clang.
> May be we ask John Myers to fix clang support for
> inline asm?
>
> -Stepan.
>
>
> Stepan Dyatkovskiy wrote:
>
> The patch. Forgot to attach it.
> -Stepan.
> Stepan Dyatkovskiy wrote:
>
> Hi Borja,
>
> > What happens in your example above when you
> use a real
> instruction
> like
>
> for example LDD? Does it still use GPR8
> regs? To me it's
> weird that if
> you use a real instruction where operands
> are clearly
> defined in the td
> file the instruction selector uses an
> invalid regclass.
>
>
> There are two kinds of inline asm support in LLVM:
> 1. You just support all the constraints and pastes
> inline-asm contents
> as-is. That's why its still allowed to use
> names like
> "some_instr".
> 2. You may expand inline-asm strings set, or in
> another
> words just parse
> it onto set of instructions. In this case you
> have to implement
> TargetLowering::____ExpandInlineAsm method.
> I'd want to start it on this week though...
>
> But first we have to get avr-libc compilable
> (perhpas I read you
> thoughts ;-) )
>
> Relative to memory constrains. I implemented
> initial version
> it can
> catch simplest cases (from test-case):
>
> @a = internal global i16 0, align 4
> @b = internal global i16 0, align 4
> define void @mem() {
> ;CHECK: some_instr Z, Y
> call void asm "some_instr $0, $1",
> "=*Q,=*Q"(i16* @a,
> i16* @b)
> ret void
> }
>
> The patch is attached.
>
> Its in my todo yet, to handle local variables.
> They could be
> emitted as
> Y+q expression. Hope to present this support
> tomorrow. So if
> 'a' and 'b'
> from example above would be local we could get
> "some_instr
> Y, Y+2"
>
> -Stepan.
>
>
>
>
>
>
>
>
|
|
From: Borja F. <bor...@gm...> - 2013-07-01 17:41:04
|
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...>
> 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...>**>
>>
>>
>> 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...>**>
>>
>>
>> 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...>**>
>>
>>
>> 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...>**>
>>
>>
>> 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...>**>
>>
>>
>> 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...>>
>>
>>
>> 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...>**>__>
>>
>> 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...>>>
>>
>> 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...>>>>
>>
>>
>> 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...>**>__>__>__>
>>
>>
>>
>> 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...>>>>>
>>
>>
>>
>>
>> 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...>>>>>
>> 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...>**>__>__>__>
>> 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>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>
|
|
From: Stepan D. <stp...@na...> - 2013-07-09 13:05:08
Attachments:
avr-inline-asm-mem-enh-2013-07-09.patch
|
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>
>
>
>
>
>
>
>
>
>
>
|
|
From: Stepan D. <stp...@na...> - 2013-07-22 17:19:29
|
Hi Borja,
> 3) Implement the a0, a1, etc... constraints.
Not sure what you mean here? a0 as first register from "a" constraint:
asm("instr %0": "a0"(var):);
Or %a0 like that:
asm volatile(
"cli"
"ld r24, %a0"
"inc r24"
"st %a0, r24"
"sei"
:
: "e" (ptr)
: "r24"
);
-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>
>
>
>
>
>
>
>
>
>
>
|
|
From: Borja F. <bor...@gm...> - 2013-07-22 18:16:21
|
Hello Stepan, 3) Yes, it's the second case. It's explained in http://www.nongnu.org/avr-libc/user-manual/inline_asm.html but in case there are any doubts basically it works the same way as multibyte constraints except that when you see an "aN", where N is a number, you should print the alternative regname. Of course this should only work for X Y and Z, not sure how gcc complains when you use another register, or maybe it ignores it and prints the normal register name. |
|
From: Stepan D. <stp...@na...> - 2013-06-27 17:45:30
|
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...>>
>
> 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...>>__>
>
>
> 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...>>>
>
>
> 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...>>>
> To: Borja Ferrer <bor...@gm...
> <mailto:bor...@gm...>
> <mailto:bor...@gm...
> <mailto:bor...@gm...>>__>
> CC: avr-llvm <avr-llvm-devel@lists.__source__forge.net
> <http://sourceforge.net>
> <mailto:avr-llvm-devel@lists.__sourceforge.net
> <mailto:avr...@li...>>>, John Myers
> <ato...@gm...
> <mailto:ato...@gm...>
> <mailto:atomicdog.jwm@gmail.__com <mailto:ato...@gm...>>>
>
> Hi Borja,
>
> 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
>
>
>
>
> 2013/6/25 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...>>>>
>
> Hi Borja.
> This is new patch. Seems I've moved memory
> constraint
> implementation
> to the same level like all other constraints. Note
> avr-gcc has buggy
> implementation of memory constraint, that's why
> *avr-libc doesn't
> use it*.
>
> I removed restriction for Y,Z in
> getLargestLegalSuperClass, so these
> registers could be inflated now. But there was
> unimplemented
> getPointerRegClass method in AVRRegistersInfo. So
> currently I have
> restricted it to Y and Z. Though suppose we
> can extend
> it in future
> to XYZ set.
> Look changes in inline-asm.ll to see what
> exactly is
> supported now.
>
> About clang.
> May be we ask John Myers to fix clang support for
> inline asm?
>
> -Stepan.
>
>
> Stepan Dyatkovskiy wrote:
>
> The patch. Forgot to attach it.
> -Stepan.
> Stepan Dyatkovskiy wrote:
>
> Hi Borja,
>
> > What happens in your example above
> when you
> use a real
> instruction
> like
>
> for example LDD? Does it still use
> GPR8
> regs? To me it's
> weird that if
> you use a real instruction where
> operands
> are clearly
> defined in the td
> file the instruction selector uses an
> invalid regclass.
>
>
> There are two kinds of inline asm
> support in LLVM:
> 1. You just support all the
> constraints and pastes
> inline-asm contents
> as-is. That's why its still allowed to use
> names like
> "some_instr".
> 2. You may expand inline-asm strings
> set, or in
> another
> words just parse
> it onto set of instructions. In this
> case you
> have to implement
> TargetLowering::______ExpandInlineAsm
> method.
>
> I'd want to start it on this week
> though...
>
> But first we have to get avr-libc
> compilable
> (perhpas I read you
> thoughts ;-) )
>
> Relative to memory constrains. I
> implemented
> initial version
> it can
> catch simplest cases (from test-case):
>
> @a = internal global i16 0, align 4
> @b = internal global i16 0, align 4
> define void @mem() {
> ;CHECK: some_instr Z, Y
> call void asm "some_instr $0, $1",
> "=*Q,=*Q"(i16* @a,
> i16* @b)
> ret void
> }
>
> The patch is attached.
>
> Its in my todo yet, to handle local
> variables.
> They could be
> emitted as
> Y+q expression. Hope to present this
> support
> tomorrow. So if
> 'a' and 'b'
> from example above would be local we
> could get
> "some_instr
> Y, Y+2"
>
> -Stepan.
>
>
>
>
>
>
>
>
>
>
|
|
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>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>
|
|
From: Stepan D. <stp...@na...> - 2013-07-10 19:15:26
Attachments:
avr-inline-asm-mem-enh-2013-07-10.patch
|
Hi Borja,
> 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?
I've compared it with ARM backend today (the only one that does
something serious with InlineAsm node).
Well all stuff I did in LowerINLINEASM is implemented in
SelectInlineAsm, in ARMISelDAGToDAG.
Well that's true, its not a lowering. I closer to the final stage, when
you just correct the things and select proper instructions.
So I did the same: I've moved everything from LowerINLINEASM to
SelectInlineAsm. The new patch is attached.
Theoretically, we can move everything related to memory constraint into
single SelectInlineAsm method. But in this case I'll need to check all
default behaviour that will be overridden. For me it is better to have
as much default behaviour as possible.
> 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.
If Op is not a MemSDNode it dyn_cast will return false. So MemSDNode is
load or store node. InlineAsm in our case could play the same role too.
If you think it is some extra code in SelectAddr I can move it to
SelectInlineAsmMemoryOperand, though I'll have to copypaste something
from SelectAddr in this case.
-Stepan.
Borja Ferrer wrote:
> 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... <mailto: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...> <mailto: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...>>
> <mailto: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...>>
> <mailto: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...>>
> <mailto: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...>>
> <mailto: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...>>
> <mailto: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...>>
> <mailto: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...>>__>
>
> <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, 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...>>>
>
> <mailto: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...>>>>
> <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...>>>>>>
>
>
>
> 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...>>__>__>__>
>
>
> <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...
> <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...>>>>>
>
>
> <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...
> <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...>>>>>
>
>
> <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...
> <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...>>__>__>__>
>
>
> <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...
> <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>
>
>
>
>
>
>
>
>
>
>
>
>
|
|
From: Stepan D. <stp...@na...> - 2013-06-27 19:03:12
Attachments:
avr-inline-asm-mem-multibyte-2013-06-27.patch
|
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://sourceforge.net>
> <mailto:avr-llvm-devel@lists.
> <mailto:avr-llvm-devel@lists.>____sourceforge.net
> <http://sourceforge.net>
>
> <mailto:avr-llvm-devel@lists.__sourceforge.net
> <mailto:avr...@li...>>>>, John Myers
> <ato...@gm...
> <mailto:ato...@gm...>
> <mailto:atomicdog.jwm@gmail.__com
> <mailto:ato...@gm...>>
> <mailto:atomicdog.jwm@gmail.
> <mailto:atomicdog.jwm@gmail.>____com
> <mailto:atomicdog.jwm@gmail.__com
> <mailto:ato...@gm...>>>>
>
>
> Hi Borja,
>
> 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
>
>
>
>
> 2013/6/25 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.
> This is new patch. Seems I've
> moved memory
> constraint
> implementation
> to the same level like all other
> constraints. Note
> avr-gcc has buggy
> implementation of memory
> constraint, that's why
> *avr-libc doesn't
> use it*.
>
> I removed restriction for Y,Z in
> getLargestLegalSuperClass, so these
> registers could be inflated now.
> But there was
> unimplemented
> getPointerRegClass method in
> AVRRegistersInfo. So
> currently I have
> restricted it to Y and Z. Though
> suppose we
> can extend
> it in future
> to XYZ set.
> Look changes in inline-asm.ll to
> see what
> exactly is
> supported now.
>
> About clang.
> May be we ask John Myers to fix
> clang support for
> inline asm?
>
> -Stepan.
>
>
> Stepan Dyatkovskiy wrote:
>
> The patch. Forgot to attach it.
> -Stepan.
> Stepan Dyatkovskiy wrote:
>
> Hi Borja,
>
> > What happens in your
> example above
> when you
> use a real
> instruction
> like
>
> for example LDD? Does
> it still use
> GPR8
> regs? To me it's
> weird that if
> you use a real
> instruction where
> operands
> are clearly
> defined in the td
> file the instruction
> selector uses an
> invalid regclass.
>
>
> There are two kinds of
> inline asm
> support in LLVM:
> 1. You just support all the
> constraints and pastes
> inline-asm contents
> as-is. That's why its
> still allowed to use
> names like
> "some_instr".
> 2. You may expand
> inline-asm strings
> set, or in
> another
> words just parse
> it onto set of
> instructions. In this
> case you
> have to implement
>
> TargetLowering::________ExpandInlineAsm
>
> method.
>
> I'd want to start it on
> this week
> though...
>
> But first we have to get
> avr-libc
> compilable
> (perhpas I read you
> thoughts ;-) )
>
> Relative to memory
> constrains. I
> implemented
> initial version
> it can
> catch simplest cases
> (from test-case):
>
> @a = internal global i16
> 0, align 4
> @b = internal global i16
> 0, align 4
> define void @mem() {
> ;CHECK: some_instr Z, Y
> call void asm
> "some_instr $0, $1",
> "=*Q,=*Q"(i16* @a,
> i16* @b)
> ret void
> }
>
> The patch is attached.
>
> Its in my todo yet, to
> handle local
> variables.
> They could be
> emitted as
> Y+q expression. Hope to
> present this
> support
> tomorrow. So if
> 'a' and 'b'
> from example above would
> be local we
> could get
> "some_instr
> Y, Y+2"
>
> -Stepan.
>
>
>
>
>
>
>
>
>
>
>
>
>
|
|
From: Borja F. <bor...@gm...> - 2013-07-11 16:02:25
|
Hello Stepan, 1) Wow, those are really big conditions xD. You could split them into single conditions and use continues: if (cond 1) continue; if (cond 2) continue; etc... instead of having a huge if() that is quite hard to read. 2) Yes I thought about that aswell, that not all constant nodes are asm flags, but the ARM backend does this, so either they have a bug there or it's safe? I prefer if you could move the big condition inside the for() into the top of the loop, it makes the for() quite unreadable. And adjust there the i variable as needed for each suboperand as you mentioned before. 4) Why do you say they handle everything in the SelectInlineAsm method? As far as i can see they only handle one specific case, otherwise they return NULL and let the default behaviour do the work. 6) Yes glues are a bit tricky, dont worry about them, I will fix them when this gets commited. One last thing, when you run the regression tests, enable locally the machine verifier to catch any additional errors. To enable it unconditionally search for the command object (i think it's declared in codegen/passes.cpp as a cl::opt) and turn it on by default. When you're doing your own tests you can enable it passing -verify-machineisntrs or something like that to llc. |
|
From: Stepan D. <stp...@na...> - 2013-07-12 13:26:37
Attachments:
avr-inline-asm-mem-enh-2013-07-12.patch
|
Hello Borja. Seems I've made things much simpler. Can't understand why I didn't it in beginning. I moved everything into SelectInlineAsmMemoryOperand hook. #1: We don't need 'for' at all. Since it implemented in hook caller. #2: This problem disappeared, see #1. #4: The same. #6: Still not sure about glues. I just tried to keep all chains in proper order. But you rather look at new SelectInlineAsmMemoryOperand. > One last thing, when you run the regression tests, enable locally the > machine verifier to catch any additional errors. To enable it > unconditionally search for the command object (i think it's declared in > codegen/passes.cpp as a cl::opt) and turn it on by default. When you're > doing your own tests you can enable it passing -verify-machineisntrs or > something like that to llc. OK. -Stepan. Borja Ferrer wrote: > Hello Stepan, > > 1) Wow, those are really big conditions xD. You could split them into > single conditions and use continues: > if (cond 1) continue; > if (cond 2) continue; > etc... > > instead of having a huge if() that is quite hard to read. > > 2) Yes I thought about that aswell, that not all constant nodes are asm > flags, but the ARM backend does this, so either they have a bug there or > it's safe? > I prefer if you could move the big condition inside the for() into the > top of the loop, it makes the for() quite unreadable. And adjust there > the i variable as needed for each suboperand as you mentioned before. > > 4) Why do you say they handle everything in the SelectInlineAsm method? > As far as i can see they only handle one specific case, otherwise they > return NULL and let the default behaviour do the work. > > 6) Yes glues are a bit tricky, dont worry about them, I will fix them > when this gets commited. > > One last thing, when you run the regression tests, enable locally the > machine verifier to catch any additional errors. To enable it > unconditionally search for the command object (i think it's declared in > codegen/passes.cpp as a cl::opt) and turn it on by default. When you're > doing your own tests you can enable it passing -verify-machineisntrs or > something like that to llc. > |
|
From: Borja F. <bor...@gm...> - 2013-07-24 14:28:05
|
Hello Stepan, The patch looks good, but I have some questions/comments: 1) You implemented the "aN" constraint by also allowing b,c,d constraints. I'm not sure if gcc implements this, my guess is that only "a" is needed. Please check to see what gcc does here. 2) Make getAlternativeRegName return a char instead of a string since we only need to return one letter. 3) Related to the changes in AVRISelLowering.cpp, do we need to care about more register constraints apart of the ones you already covered? 4) Now that wider types have been covered, please check if the assert I commented out in AVRISelLowering.cpp in the inline asm code can be removed or what. 2013/7/24 Stepan Dyatkovskiy <stp...@na...> > Hello Borja, > Here is the new patch for multibyte reference. Please look at new tests to > see what was implemented exactly. > To fix issue I described in previous letter I've added new reg classes > that contains pairs of 8 bit registers: > If we pass i32 parameter that should be split onto four 8 bit regs, we set > i16 register class first, then it would be replaced with 8 bit registers if > needed. Currently I see it is the only solution that doesn't touch vmcore. > > -Stepan. > > Stepan Dyatkovskiy wrote: > >> Hello Borja, >> Great. I have found out next inline-asm issue. >> When you pass i32 bit type, it could be split onto two registers only, >> no matter which register class you set for it. This behaviour is >> implemented in SelectionDAGBuilder::**visitInlineAsm. I think I'll >> implement simplest version of ExpandInlineAsm today, that will fix >> operand types properly. >> >> -Stepan >> >> Borja Ferrer wrote: >> >>> I've been adding fixes to pass a ton of regression in the Generic folder. >>> Currently there are two tests that fail with a "cannot select >>> build_pair" related to adding 16bit reg classes in the inline asm code. >>> The rest of failing tests are related with not being able to run llc >>> with -O0. There's a serious codegen bug that i spotted yesterday while >>> doing the fixes that I need to work on. >>> >>> >>> 2013/7/22 Borja Ferrer <bor...@gm... >>> <mailto:bor...@gm...>**> >>> >>> Hello Stepan, >>> >>> 3) Yes, it's the second case. It's explained in >>> http://www.nongnu.org/avr-**libc/user-manual/inline_asm.**html<http://www.nongnu.org/avr-libc/user-manual/inline_asm.html> >>> but in case there are any doubts basically it works the same way as >>> multibyte constraints except that when you see an "aN", where N is a >>> number, you should print the alternative regname. Of course this >>> should only work for X Y and Z, not sure how gcc complains when you >>> use another register, or maybe it ignores it and prints the normal >>> register name. >>> >>> >>> >> >> ------------------------------**------------------------------** >> ------------------ >> See everything from the browser to the database with AppDynamics >> Get end-to-end visibility with application monitoring from AppDynamics >> Isolate bottlenecks and diagnose root cause in seconds. >> Start your free trial of AppDynamics Pro today! >> http://pubads.g.doubleclick.**net/gampad/clk?id=48808831&iu=** >> /4140/ostg.clktrk<http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk> >> ______________________________**_________________ >> avr-llvm-devel mailing list >> avr-llvm-devel@lists.**sourceforge.net<avr...@li...> >> https://lists.sourceforge.net/**lists/listinfo/avr-llvm-devel<https://lists.sourceforge.net/lists/listinfo/avr-llvm-devel> >> >> > |
|
From: Stepan D. <stp...@na...> - 2013-07-24 15:00:18
Attachments:
avr-inline-asm-multibyte-2013-07-24-2.patch
|
Hi Borja, > 1) You implemented the "aN" constraint by also allowing b,c,d > constraints. I'm not sure if gcc implements this, my guess is that only > "a" is needed. Please check to see what gcc does here. For b,c,d avr-gcc acts like it meet %a. I think its wrong and untested behaviour, so I just restricted usage to single 'a' character. > 2) Make getAlternativeRegName return a char instead of a string since we > only need to return one letter. Done > 3) Related to the changes in AVRISelLowering.cpp, do we need to care > about more register constraints apart of the ones you already covered? What you mean? I think all constraints are covered now. If no, we can find out it during compilation (avr-libc, arduino, etc). > 4) Now that wider types have been covered, please check if the assert I > commented out in AVRISelLowering.cpp in the inline asm code can be > removed or what. I replaced this assertion with error message. Suppose its better than killing the compiler :-) New patch is attached. -Stepan. Borja Ferrer wrote: > Hello Stepan, > The patch looks good, but I have some questions/comments: > > 1) You implemented the "aN" constraint by also allowing b,c,d > constraints. I'm not sure if gcc implements this, my guess is that only > "a" is needed. Please check to see what gcc does here. > 2) Make getAlternativeRegName return a char instead of a string since we > only need to return one letter. > 3) Related to the changes in AVRISelLowering.cpp, do we need to care > about more register constraints apart of the ones you already covered? > 4) Now that wider types have been covered, please check if the assert I > commented out in AVRISelLowering.cpp in the inline asm code can be > removed or what. > > > > 2013/7/24 Stepan Dyatkovskiy <stp...@na... <mailto:stp...@na...>> > > Hello Borja, > Here is the new patch for multibyte reference. Please look at new > tests to see what was implemented exactly. > To fix issue I described in previous letter I've added new reg > classes that contains pairs of 8 bit registers: > If we pass i32 parameter that should be split onto four 8 bit regs, > we set i16 register class first, then it would be replaced with 8 > bit registers if needed. Currently I see it is the only solution > that doesn't touch vmcore. > > -Stepan. > > Stepan Dyatkovskiy wrote: > > Hello Borja, > Great. I have found out next inline-asm issue. > When you pass i32 bit type, it could be split onto two registers > only, > no matter which register class you set for it. This behaviour is > implemented in SelectionDAGBuilder::__visitInlineAsm. I think I'll > implement simplest version of ExpandInlineAsm today, that will fix > operand types properly. > > -Stepan > > Borja Ferrer wrote: > > I've been adding fixes to pass a ton of regression in the > Generic folder. > Currently there are two tests that fail with a "cannot select > build_pair" related to adding 16bit reg classes in the > inline asm code. > The rest of failing tests are related with not being able to > run llc > with -O0. There's a serious codegen bug that i spotted > yesterday while > doing the fixes that I need to work on. > > > 2013/7/22 Borja Ferrer <bor...@gm... > <mailto:bor...@gm...> > <mailto:bor...@gm... <mailto:bor...@gm...>>__> > > Hello Stepan, > > 3) Yes, it's the second case. It's explained in > http://www.nongnu.org/avr-__libc/user-manual/inline_asm.__html > <http://www.nongnu.org/avr-libc/user-manual/inline_asm.html> > but in case there are any doubts basically it works > the same way as > multibyte constraints except that when you see an > "aN", where N is a > number, you should print the alternative regname. Of > course this > should only work for X Y and Z, not sure how gcc > complains when you > use another register, or maybe it ignores it and > prints the normal > register name. > > > > > ------------------------------__------------------------------__------------------ > See everything from the browser to the database with AppDynamics > Get end-to-end visibility with application monitoring from > AppDynamics > Isolate bottlenecks and diagnose root cause in seconds. > Start your free trial of AppDynamics Pro today! > http://pubads.g.doubleclick.__net/gampad/clk?id=48808831&iu=__/4140/ostg.clktrk > <http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk> > _________________________________________________ > avr-llvm-devel mailing list > avr-llvm-devel@lists.__sourceforge.net > <mailto:avr...@li...> > https://lists.sourceforge.net/__lists/listinfo/avr-llvm-devel > <https://lists.sourceforge.net/lists/listinfo/avr-llvm-devel> > > > |
|
From: Stepan D. <stp...@na...> - 2013-06-28 18:44:25
Attachments:
avr-inline-asm-multibyte-2013-06-28.patch
|
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 <http://sourceforge.net>
>
|
|
From: Borja F. <bor...@gm...> - 2013-07-12 15:00:38
|
Ohhh this is a lot better, atleast fixing patches iteratively brought us to a nice solution :) If this patch passes all tests with the machine verifier enabled then feel free to commit it. I'll take a look at the glue operands after this. 2013/7/12 Stepan Dyatkovskiy <stp...@na...> > Hello Borja. > Seems I've made things much simpler. Can't understand why I didn't it in > beginning. > > I moved everything into SelectInlineAsmMemoryOperand hook. > #1: We don't need 'for' at all. Since it implemented in hook caller. > #2: This problem disappeared, see #1. > #4: The same. > #6: Still not sure about glues. I just tried to keep all chains in proper > order. But you rather look at new SelectInlineAsmMemoryOperand. > > > > One last thing, when you run the regression tests, enable locally the > > machine verifier to catch any additional errors. To enable it > > unconditionally search for the command object (i think it's declared in > > codegen/passes.cpp as a cl::opt) and turn it on by default. When you're > > doing your own tests you can enable it passing -verify-machineisntrs or > > something like that to llc. > OK. > > -Stepan. > > > Borja Ferrer wrote: > >> Hello Stepan, >> >> 1) Wow, those are really big conditions xD. You could split them into >> single conditions and use continues: >> if (cond 1) continue; >> if (cond 2) continue; >> etc... >> >> instead of having a huge if() that is quite hard to read. >> >> 2) Yes I thought about that aswell, that not all constant nodes are asm >> flags, but the ARM backend does this, so either they have a bug there or >> it's safe? >> I prefer if you could move the big condition inside the for() into the >> top of the loop, it makes the for() quite unreadable. And adjust there >> the i variable as needed for each suboperand as you mentioned before. >> >> 4) Why do you say they handle everything in the SelectInlineAsm method? >> As far as i can see they only handle one specific case, otherwise they >> return NULL and let the default behaviour do the work. >> >> 6) Yes glues are a bit tricky, dont worry about them, I will fix them >> when this gets commited. >> >> One last thing, when you run the regression tests, enable locally the >> machine verifier to catch any additional errors. To enable it >> unconditionally search for the command object (i think it's declared in >> codegen/passes.cpp as a cl::opt) and turn it on by default. When you're >> doing your own tests you can enable it passing -verify-machineisntrs or >> something like that to llc. >> >> > |
|
From: Stepan D. <stp...@na...> - 2013-07-15 12:04:53
|
r269 -Stepan Borja Ferrer wrote: > Ohhh this is a lot better, atleast fixing patches iteratively brought us > to a nice solution :) > > If this patch passes all tests with the machine verifier enabled then > feel free to commit it. I'll take a look at the glue operands after this. > > > 2013/7/12 Stepan Dyatkovskiy <stp...@na... <mailto:stp...@na...>> > > Hello Borja. > Seems I've made things much simpler. Can't understand why I didn't > it in beginning. > > I moved everything into SelectInlineAsmMemoryOperand hook. > #1: We don't need 'for' at all. Since it implemented in hook caller. > #2: This problem disappeared, see #1. > #4: The same. > #6: Still not sure about glues. I just tried to keep all chains in > proper order. But you rather look at new SelectInlineAsmMemoryOperand. > > > > One last thing, when you run the regression tests, enable locally the > > machine verifier to catch any additional errors. To enable it > > unconditionally search for the command object (i think it's > declared in > > codegen/passes.cpp as a cl::opt) and turn it on by default. When > you're > > doing your own tests you can enable it passing > -verify-machineisntrs or > > something like that to llc. > OK. > > -Stepan. > > > Borja Ferrer wrote: > > Hello Stepan, > > 1) Wow, those are really big conditions xD. You could split them > into > single conditions and use continues: > if (cond 1) continue; > if (cond 2) continue; > etc... > > instead of having a huge if() that is quite hard to read. > > 2) Yes I thought about that aswell, that not all constant nodes > are asm > flags, but the ARM backend does this, so either they have a bug > there or > it's safe? > I prefer if you could move the big condition inside the for() > into the > top of the loop, it makes the for() quite unreadable. And adjust > there > the i variable as needed for each suboperand as you mentioned > before. > > 4) Why do you say they handle everything in the SelectInlineAsm > method? > As far as i can see they only handle one specific case, > otherwise they > return NULL and let the default behaviour do the work. > > 6) Yes glues are a bit tricky, dont worry about them, I will fix > them > when this gets commited. > > One last thing, when you run the regression tests, enable > locally the > machine verifier to catch any additional errors. To enable it > unconditionally search for the command object (i think it's > declared in > codegen/passes.cpp as a cl::opt) and turn it on by default. When > you're > doing your own tests you can enable it passing > -verify-machineisntrs or > something like that to llc. > > > |
|
From: Borja F. <bor...@gm...> - 2013-07-24 15:10:40
|
1) I agree. 3) What happens when you use multibyte constraints with "e" or "b". 4) Hah ok, btw no need to check for i64? 2013/7/24 Stepan Dyatkovskiy <stp...@na...> > Hi Borja, > > > > 1) You implemented the "aN" constraint by also allowing b,c,d > > constraints. I'm not sure if gcc implements this, my guess is that only > > "a" is needed. Please check to see what gcc does here. > > For b,c,d avr-gcc acts like it meet %a. I think its wrong and untested > behaviour, so I just restricted usage to single 'a' character. > > > > 2) Make getAlternativeRegName return a char instead of a string since we > > only need to return one letter. > > Done > > > > 3) Related to the changes in AVRISelLowering.cpp, do we need to care > > about more register constraints apart of the ones you already covered? > > What you mean? I think all constraints are covered now. If no, we can find > out it during compilation (avr-libc, arduino, etc). > > > > 4) Now that wider types have been covered, please check if the assert I > > commented out in AVRISelLowering.cpp in the inline asm code can be > > removed or what. > > I replaced this assertion with error message. Suppose its better than > killing the compiler :-) > > New patch is attached. > > -Stepan. > > > Borja Ferrer wrote: > >> Hello Stepan, >> The patch looks good, but I have some questions/comments: >> >> 1) You implemented the "aN" constraint by also allowing b,c,d >> constraints. I'm not sure if gcc implements this, my guess is that only >> "a" is needed. Please check to see what gcc does here. >> 2) Make getAlternativeRegName return a char instead of a string since we >> only need to return one letter. >> 3) Related to the changes in AVRISelLowering.cpp, do we need to care >> about more register constraints apart of the ones you already covered? >> 4) Now that wider types have been covered, please check if the assert I >> commented out in AVRISelLowering.cpp in the inline asm code can be >> removed or what. >> >> >> >> 2013/7/24 Stepan Dyatkovskiy <stp...@na... <mailto:stp...@na... >> >> >> >> >> Hello Borja, >> Here is the new patch for multibyte reference. Please look at new >> tests to see what was implemented exactly. >> To fix issue I described in previous letter I've added new reg >> classes that contains pairs of 8 bit registers: >> If we pass i32 parameter that should be split onto four 8 bit regs, >> we set i16 register class first, then it would be replaced with 8 >> bit registers if needed. Currently I see it is the only solution >> that doesn't touch vmcore. >> >> -Stepan. >> >> Stepan Dyatkovskiy wrote: >> >> Hello Borja, >> Great. I have found out next inline-asm issue. >> When you pass i32 bit type, it could be split onto two registers >> only, >> no matter which register class you set for it. This behaviour is >> implemented in SelectionDAGBuilder::__**visitInlineAsm. I think >> I'll >> >> implement simplest version of ExpandInlineAsm today, that will fix >> operand types properly. >> >> -Stepan >> >> Borja Ferrer wrote: >> >> I've been adding fixes to pass a ton of regression in the >> Generic folder. >> Currently there are two tests that fail with a "cannot select >> build_pair" related to adding 16bit reg classes in the >> inline asm code. >> The rest of failing tests are related with not being able to >> run llc >> with -O0. There's a serious codegen bug that i spotted >> yesterday while >> doing the fixes that I need to work on. >> >> >> 2013/7/22 Borja Ferrer <bor...@gm... >> <mailto:bor...@gm...> >> <mailto:bor...@gm... <mailto:bor...@gm...>* >> *>__> >> >> Hello Stepan, >> >> 3) Yes, it's the second case. It's explained in >> http://www.nongnu.org/avr-__**libc/user-manual/inline_asm.__* >> *html <http://www.nongnu.org/avr-__libc/user-manual/inline_asm.__html> >> >> <http://www.nongnu.org/avr-**libc/user-manual/inline_asm.** >> html <http://www.nongnu.org/avr-libc/user-manual/inline_asm.html>> >> but in case there are any doubts basically it works >> the same way as >> multibyte constraints except that when you see an >> "aN", where N is a >> number, you should print the alternative regname. Of >> course this >> should only work for X Y and Z, not sure how gcc >> complains when you >> use another register, or maybe it ignores it and >> prints the normal >> register name. >> >> >> >> >> ------------------------------**__----------------------------** >> --__------------------ >> >> See everything from the browser to the database with AppDynamics >> Get end-to-end visibility with application monitoring from >> AppDynamics >> Isolate bottlenecks and diagnose root cause in seconds. >> Start your free trial of AppDynamics Pro today! >> http://pubads.g.doubleclick.__**net/gampad/clk?id=48808831&iu=** >> __/4140/ostg.clktrk >> <http://pubads.g.doubleclick.**net/gampad/clk?id=48808831&iu=** >> /4140/ostg.clktrk<http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk> >> > >> ______________________________**___________________ >> avr-llvm-devel mailing list >> avr-llvm-devel@lists.__sourcef**orge.net <http://sourceforge.net> >> <mailto:avr-llvm-devel@lists.**sourceforge.net<avr...@li...> >> > >> https://lists.sourceforge.net/**__lists/listinfo/avr-llvm-**devel<https://lists.sourceforge.net/__lists/listinfo/avr-llvm-devel> >> <https://lists.sourceforge.**net/lists/listinfo/avr-llvm-**devel<https://lists.sourceforge.net/lists/listinfo/avr-llvm-devel> >> > >> >> >> >> > |
|
From: Stepan D. <stp...@na...> - 2013-07-24 15:58:57
|
Borja Ferrer wrote: > 1) I agree. > 3) What happens when you use multibyte constraints with "e" or "b". Just had checked it. It works :-) Though I'll add test-cases for them too. > 4) Hah ok, btw no need to check for i64? In one of our previous mails we decided to restrict i64 usage, since avr-gcc doesn't support it properly. -Stepan > > > 2013/7/24 Stepan Dyatkovskiy <stp...@na... <mailto:stp...@na...>> > > Hi Borja, > > > > 1) You implemented the "aN" constraint by also allowing b,c,d > > constraints. I'm not sure if gcc implements this, my guess is > that only > > "a" is needed. Please check to see what gcc does here. > > For b,c,d avr-gcc acts like it meet %a. I think its wrong and > untested behaviour, so I just restricted usage to single 'a' character. > > > > 2) Make getAlternativeRegName return a char instead of a string > since we > > only need to return one letter. > > Done > > > > 3) Related to the changes in AVRISelLowering.cpp, do we need to care > > about more register constraints apart of the ones you already > covered? > > What you mean? I think all constraints are covered now. If no, we > can find out it during compilation (avr-libc, arduino, etc). > > > > 4) Now that wider types have been covered, please check if the > assert I > > commented out in AVRISelLowering.cpp in the inline asm code can be > > removed or what. > > I replaced this assertion with error message. Suppose its better > than killing the compiler :-) > > New patch is attached. > > -Stepan. > > > Borja Ferrer wrote: > > Hello Stepan, > The patch looks good, but I have some questions/comments: > > 1) You implemented the "aN" constraint by also allowing b,c,d > constraints. I'm not sure if gcc implements this, my guess is > that only > "a" is needed. Please check to see what gcc does here. > 2) Make getAlternativeRegName return a char instead of a string > since we > only need to return one letter. > 3) Related to the changes in AVRISelLowering.cpp, do we need to care > about more register constraints apart of the ones you already > covered? > 4) Now that wider types have been covered, please check if the > assert I > commented out in AVRISelLowering.cpp in the inline asm code can be > removed or what. > > > > 2013/7/24 Stepan Dyatkovskiy <stp...@na... > <mailto:stp...@na...> <mailto:stp...@na... > <mailto:stp...@na...>>> > > > Hello Borja, > Here is the new patch for multibyte reference. Please look > at new > tests to see what was implemented exactly. > To fix issue I described in previous letter I've added new reg > classes that contains pairs of 8 bit registers: > If we pass i32 parameter that should be split onto four 8 > bit regs, > we set i16 register class first, then it would be replaced > with 8 > bit registers if needed. Currently I see it is the only > solution > that doesn't touch vmcore. > > -Stepan. > > Stepan Dyatkovskiy wrote: > > Hello Borja, > Great. I have found out next inline-asm issue. > When you pass i32 bit type, it could be split onto two > registers > only, > no matter which register class you set for it. This > behaviour is > implemented in SelectionDAGBuilder::____visitInlineAsm. > I think I'll > > implement simplest version of ExpandInlineAsm today, > that will fix > operand types properly. > > -Stepan > > Borja Ferrer wrote: > > I've been adding fixes to pass a ton of regression > in the > Generic folder. > Currently there are two tests that fail with a > "cannot select > build_pair" related to adding 16bit reg classes in the > inline asm code. > The rest of failing tests are related with not > being able to > run llc > with -O0. There's a serious codegen bug that i spotted > yesterday while > doing the fixes that I need to work on. > > > 2013/7/22 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...>>__>__> > > Hello Stepan, > > 3) Yes, it's the second case. It's explained in > http://www.nongnu.org/avr-____libc/user-manual/inline_asm.____html > <http://www.nongnu.org/avr-__libc/user-manual/inline_asm.__html> > > > <http://www.nongnu.org/avr-__libc/user-manual/inline_asm.__html > <http://www.nongnu.org/avr-libc/user-manual/inline_asm.html>> > but in case there are any doubts basically it > works > the same way as > multibyte constraints except that when you see an > "aN", where N is a > number, you should print the alternative > regname. Of > course this > should only work for X Y and Z, not sure how gcc > complains when you > use another register, or maybe it ignores it and > prints the normal > register name. > > > > > > ------------------------------____----------------------------__--__------------------ > > See everything from the browser to the database with > AppDynamics > Get end-to-end visibility with application monitoring from > AppDynamics > Isolate bottlenecks and diagnose root cause in seconds. > Start your free trial of AppDynamics Pro today! > http://pubads.g.doubleclick.____net/gampad/clk?id=48808831&iu=____/4140/ostg.clktrk > > <http://pubads.g.doubleclick.__net/gampad/clk?id=48808831&iu=__/4140/ostg.clktrk > <http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk>> > ___________________________________________________ > avr-llvm-devel mailing list > avr-llvm-devel@lists.__sourcef__orge.net > <http://sourceforge.net> > <mailto:avr-llvm-devel@lists.__sourceforge.net > <mailto:avr...@li...>> > https://lists.sourceforge.net/____lists/listinfo/avr-llvm-__devel <https://lists.sourceforge.net/__lists/listinfo/avr-llvm-devel> > > <https://lists.sourceforge.__net/lists/listinfo/avr-llvm-__devel > <https://lists.sourceforge.net/lists/listinfo/avr-llvm-devel>> > > > > > |
|
From: Borja F. <bor...@gm...> - 2013-07-24 16:56:14
|
Ok, you can commit the patch now. 2013/7/24 Stepan Dyatkovskiy <stp...@na...> > Borja Ferrer wrote: > >> 1) I agree. >> > > 3) What happens when you use multibyte constraints with "e" or "b". >> > Just had checked it. It works :-) Though I'll add test-cases for them too. > > > 4) Hah ok, btw no need to check for i64? >> > In one of our previous mails we decided to restrict i64 usage, since > avr-gcc doesn't support it properly. > > -Stepan > > >> >> 2013/7/24 Stepan Dyatkovskiy <stp...@na... <mailto:stp...@na... >> >> >> >> Hi Borja, >> >> >> > 1) You implemented the "aN" constraint by also allowing b,c,d >> > constraints. I'm not sure if gcc implements this, my guess is >> that only >> > "a" is needed. Please check to see what gcc does here. >> >> For b,c,d avr-gcc acts like it meet %a. I think its wrong and >> untested behaviour, so I just restricted usage to single 'a' >> character. >> >> >> > 2) Make getAlternativeRegName return a char instead of a string >> since we >> > only need to return one letter. >> >> Done >> >> >> > 3) Related to the changes in AVRISelLowering.cpp, do we need to >> care >> > about more register constraints apart of the ones you already >> covered? >> >> What you mean? I think all constraints are covered now. If no, we >> can find out it during compilation (avr-libc, arduino, etc). >> >> >> > 4) Now that wider types have been covered, please check if the >> assert I >> > commented out in AVRISelLowering.cpp in the inline asm code can be >> > removed or what. >> >> I replaced this assertion with error message. Suppose its better >> than killing the compiler :-) >> >> New patch is attached. >> >> -Stepan. >> >> >> Borja Ferrer wrote: >> >> Hello Stepan, >> The patch looks good, but I have some questions/comments: >> >> 1) You implemented the "aN" constraint by also allowing b,c,d >> constraints. I'm not sure if gcc implements this, my guess is >> that only >> "a" is needed. Please check to see what gcc does here. >> 2) Make getAlternativeRegName return a char instead of a string >> since we >> only need to return one letter. >> 3) Related to the changes in AVRISelLowering.cpp, do we need to >> care >> about more register constraints apart of the ones you already >> covered? >> 4) Now that wider types have been covered, please check if the >> assert I >> commented out in AVRISelLowering.cpp in the inline asm code can be >> removed or what. >> >> >> >> 2013/7/24 Stepan Dyatkovskiy <stp...@na... >> <mailto:stp...@na...> <mailto:stp...@na... >> >> <mailto:stp...@na...>>> >> >> >> Hello Borja, >> Here is the new patch for multibyte reference. Please look >> at new >> tests to see what was implemented exactly. >> To fix issue I described in previous letter I've added new >> reg >> classes that contains pairs of 8 bit registers: >> If we pass i32 parameter that should be split onto four 8 >> bit regs, >> we set i16 register class first, then it would be replaced >> with 8 >> bit registers if needed. Currently I see it is the only >> solution >> that doesn't touch vmcore. >> >> -Stepan. >> >> Stepan Dyatkovskiy wrote: >> >> Hello Borja, >> Great. I have found out next inline-asm issue. >> When you pass i32 bit type, it could be split onto two >> registers >> only, >> no matter which register class you set for it. This >> behaviour is >> implemented in SelectionDAGBuilder::____** >> visitInlineAsm. >> >> I think I'll >> >> implement simplest version of ExpandInlineAsm today, >> that will fix >> operand types properly. >> >> -Stepan >> >> Borja Ferrer wrote: >> >> I've been adding fixes to pass a ton of regression >> in the >> Generic folder. >> Currently there are two tests that fail with a >> "cannot select >> build_pair" related to adding 16bit reg classes in >> the >> inline asm code. >> The rest of failing tests are related with not >> being able to >> run llc >> with -O0. There's a serious codegen bug that i >> spotted >> yesterday while >> doing the fixes that I need to work on. >> >> >> 2013/7/22 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...>**>__>__> >> >> >> Hello Stepan, >> >> 3) Yes, it's the second case. It's explained in >> http://www.nongnu.org/avr-____**libc/user-manual/inline_asm.__** >> __html<http://www.nongnu.org/avr-____libc/user-manual/inline_asm.____html> >> <http://www.nongnu.org/avr-__**libc/user-manual/inline_asm.__** >> html <http://www.nongnu.org/avr-__libc/user-manual/inline_asm.__html>> >> >> >> >> <http://www.nongnu.org/avr-__**libc/user-manual/inline_asm.__** >> html <http://www.nongnu.org/avr-__libc/user-manual/inline_asm.__html> >> <http://www.nongnu.org/avr-**libc/user-manual/inline_asm.**html<http://www.nongnu.org/avr-libc/user-manual/inline_asm.html> >> >> >> but in case there are any doubts basically it >> works >> the same way as >> multibyte constraints except that when you see >> an >> "aN", where N is a >> number, you should print the alternative >> regname. Of >> course this >> should only work for X Y and Z, not sure how >> gcc >> complains when you >> use another register, or maybe it ignores it >> and >> prints the normal >> register name. >> >> >> >> >> >> ------------------------------**____--------------------------** >> --__--__------------------ >> >> >> See everything from the browser to the database with >> AppDynamics >> Get end-to-end visibility with application monitoring >> from >> AppDynamics >> Isolate bottlenecks and diagnose root cause in seconds. >> Start your free trial of AppDynamics Pro today! >> http://pubads.g.doubleclick.__**__net/gampad/clk?id=48808831&** >> iu=____/4140/ostg.clktrk >> >> <http://pubads.g.doubleclick._**_net/gampad/clk?id=48808831&** >> iu=__/4140/ostg.clktrk >> <http://pubads.g.doubleclick.**net/gampad/clk?id=48808831&iu=** >> /4140/ostg.clktrk<http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk> >> >> >> ______________________________**_____________________ >> avr-llvm-devel mailing list >> avr-llvm-devel@lists.__sourcef**__orge.net<http://sourcef__orge.net> >> <http://sourceforge.net> >> <mailto:avr-llvm-devel@lists._**_sourceforge.net >> <mailto:avr-llvm-devel@lists.**sourceforge.net<avr...@li...> >> >> >> https://lists.sourceforge.net/**____lists/listinfo/avr-llvm-__** >> devel <https://lists.sourceforge.net/____lists/listinfo/avr-llvm-__devel>< >> https://lists.sourceforge.**net/__lists/listinfo/avr-llvm-**devel<https://lists.sourceforge.net/__lists/listinfo/avr-llvm-devel> >> > >> >> <https://lists.sourceforge.__**net/lists/listinfo/avr-llvm-__** >> devel >> <https://lists.sourceforge.**net/lists/listinfo/avr-llvm-**devel<https://lists.sourceforge.net/lists/listinfo/avr-llvm-devel> >> >> >> >> >> >> >> >> > |
|
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
>>>>>>> >
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
|
|
From: Stepan D. <stp...@na...> - 2013-07-01 17:21:39
|
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...>>
>
> 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...>>
>
> 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...>>
>
> 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...>>
>
> 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...>>
>
> 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...>>
>
> 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...>>__>
>
> 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...>>>
>
> 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...>>>>
>
>
> 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...>>__>__>__>
>
>
> 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...>>>>>
>
>
>
>
> 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...>>>>>
> 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...>>__>__>__>
> 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>
>
>
>
>
>
>
>
>
|
|
From: Borja F. <bor...@gm...> - 2013-07-17 14:07:39
|
Ok, if this will work with other reg constraints then add some tests to
cover them aswell.
What errors are you getting exactly with avrlibc? IIRC there's an assert in
llc that fires when running the greedy regalloc in O0. I dont know the
reason for this assert and if it can be safely removed, this has to be
asked in the dev list and also get some possible solutions.
2013/7/17 Stepan Dyatkovskiy <stp...@na...>
> Hi Borja.
> It should work for all other constraints as well. I don't like idea of
> fixing clang too. So we can just update getRegForInlineAsmConstraint method.
>
> I'd tried to compile avr-libc. Currently clang doesn't want to compile
> everything with -O0. While configure script tests everything with -O0. Do
> you have any ideas how to fix -O0 ?
>
> -Stepan.
>
> Borja Ferrer wrote:
>
>> Also, I guess doing only this for r is not enough, you can use
>> multibytes with other register constraints.
>>
>>
>> 2013/7/16 Borja Ferrer <bor...@gm...
>> <mailto:bor...@gm...>**>
>>
>>
>> What are you suggesting to do in clang?
>>
>>
>> 2013/7/16 Stepan Dyatkovskiy <stp...@na...
>> <mailto:stp...@na...>>
>>
>>
>> Back to multibyte referenced.
>>
>> I've attached patch with this feature + unit-test. There is some
>> question though.
>> Currently, .c code:
>> int a;
>> void f() {
>> asm("instr %A0 %B0": : "r"(a));
>> }
>> is transformed into IR below:
>> define void @f(i16 %a) {
>> entry:
>> call void asm sideeffect "instr ${0:A} ${0:B}", "r"(i16 %a)
>> ret void
>> }
>> To prevent the crash I had to fix getRegForInlineAsmConstraint,
>> I set it to return DREGS when it see i16 type or bigger. Though,
>> may be you want to fix clang and replace "r" constraint with
>> another one.
>>
>> -Stepan.
>>
>> Borja Ferrer wrote:
>>
>> Just remembered after hitting resend that this has to be
>> done always,
>> similarly like in SelectAddr(). However in this case it can
>> be done
>> unconditionally since imm is guaranteed to fit in 6bits, so
>> even if it
>> comes in a constant node of type i64 it's safe to truncate
>> it to i8.
>>
>>
>> 2013/7/15 Stepan Dyatkovskiy <stp...@na...
>> <mailto:stp...@na...> <mailto:stp...@na...
>>
>> <mailto:stp...@na...>>>
>>
>> Hi Borja,
>>
>>
>> > + CanHandleRegImmOpt &= ImmNode != 0;
>> > + CanHandleRegImmOpt &=
>> ImmNode->getAPIntValue().____**getZExtValue() < 64;
>>
>>
>> You absolutely right here. Thanks.
>>
>>
>> > There's a piece of code i initially removed because
>> i found it
>> redundant:
>> > + if (ImmNode->getValueType(0) != MVT::i8)
>> > + {
>> > + Disp = CurDAG->getTargetConstant(
>> > +
>> ImmNode->getAPIntValue().____**getZExtValue(), MVT::i8);
>>
>> > + }
>> > + else
>> > + {
>> > + Disp = ImmOp;
>> > + }
>>
>> Initially did the same: just "Disp = ImmOp", and got
>> the crash. I
>> don't remember what crash says exactly. Currently, I'm
>> not able to
>> compile anything. What I had found out is that Disp (aka
>> displacement) should be i8 only. Our back-end just
>> don't expect
>> other types of displacements.
>>
>> -Stepan.
>>
>> Borja Ferrer wrote:
>>
>> Thanks!
>>
>> I've been cleaning a bit the code, and fixed a
>> possible crash here:
>>
>> + CanHandleRegImmOpt &= ImmNode != 0;
>> + CanHandleRegImmOpt &=
>> ImmNode->getAPIntValue().____**getZExtValue() < 64;
>>
>>
>> Since ImmNode comes from a dyncast, if it's NULL
>> the second line
>> would crash.
>>
>>
>>
>> There's a piece of code i initially removed because
>> i found it
>> redundant:
>> + if (ImmNode->getValueType(0) != MVT::i8)
>> + {
>> + Disp = CurDAG->getTargetConstant(
>> +
>> ImmNode->getAPIntValue().____**getZExtValue(), MVT::i8);
>>
>> + }
>> + else
>> + {
>> + Disp = ImmOp;
>> + }
>>
>> but without im getting crashes. Can you explain why
>> this happens
>> and is needed?
>>
>>
>>
>> 2013/7/15 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...
>> >>>>
>>
>>
>> r269
>> -Stepan
>> Borja Ferrer wrote:
>>
>> Ohhh this is a lot better, atleast fixing
>> patches
>> iteratively
>> brought us
>> to a nice solution :)
>>
>> If this patch passes all tests with the
>> machine
>> verifier enabled
>> then
>> feel free to commit it. I'll take a look
>> at the glue
>> operands
>> after this.
>>
>>
>> 2013/7/12 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...>>>>>
>>
>>
>> Hello Borja.
>> Seems I've made things much simpler.
>> Can't
>> understand why I
>> didn't
>> it in beginning.
>>
>> I moved everything into
>> SelectInlineAsmMemoryOperand hook.
>> #1: We don't need 'for' at all. Since
>> it
>> implemented in
>> hook caller.
>> #2: This problem disappeared, see #1.
>> #4: The same.
>> #6: Still not sure about glues. I
>> just tried to
>> keep all
>> chains in
>> proper order. But you rather look at
>> new
>> SelectInlineAsmMemoryOperand.
>>
>>
>> > One last thing, when you run the
>> regression tests,
>> enable locally the
>> > machine verifier to catch any
>> additional errors. To
>> enable it
>> > unconditionally search for the
>> command object
>> (i think it's
>> declared in
>> > codegen/passes.cpp as a cl::opt)
>> and turn it on by
>> default. When
>> you're
>> > doing your own tests you can
>> enable it passing
>> -verify-machineisntrs or
>> > something like that to llc.
>> OK.
>>
>> -Stepan.
>>
>>
>> Borja Ferrer wrote:
>>
>> Hello Stepan,
>>
>> 1) Wow, those are really big
>> conditions xD.
>> You could
>> split them
>> into
>> single conditions and use
>> continues:
>> if (cond 1) continue;
>> if (cond 2) continue;
>> etc...
>>
>> instead of having a huge if()
>> that is quite
>> hard to read.
>>
>> 2) Yes I thought about that
>> aswell, that not all
>> constant nodes
>> are asm
>> flags, but the ARM backend does
>> this, so
>> either they
>> have a bug
>> there or
>> it's safe?
>> I prefer if you could move the
>> big condition
>> inside the
>> for()
>> into the
>> top of the loop, it makes the
>> for() quite
>> unreadable.
>> And adjust
>> there
>> the i variable as needed for each
>> suboperand
>> as you
>> mentioned
>> before.
>>
>> 4) Why do you say they handle
>> everything in the
>> SelectInlineAsm
>> method?
>> As far as i can see they only
>> handle one
>> specific case,
>> otherwise they
>> return NULL and let the default
>> behaviour do
>> the work.
>>
>> 6) Yes glues are a bit tricky,
>> dont worry
>> about them, I
>> will fix
>> them
>> when this gets commited.
>>
>> One last thing, when you run the
>> regression
>> tests, enable
>> locally the
>> machine verifier to catch any
>> additional
>> errors. To
>> enable it
>> unconditionally search for the
>> command object
>> (i think it's
>> declared in
>> codegen/passes.cpp as a cl::opt)
>> and turn it on by
>> default. When
>> you're
>> doing your own tests you can
>> enable it passing
>> -verify-machineisntrs or
>> something like that to llc.
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>
|
|
From: Stepan D. <stp...@na...> - 2013-07-18 11:12:16
Attachments:
fclose-29b051.s
|
Hi Borja,
Well, ok. I will write several tests for it. Though I think also about
avr-libc compilation. Today I've made two small fixes in this direction:
1. Are there any reasons to forbid default FastRA for -O0? Note,
currently for -O0 it doesn't work at all. So I've temoporary restored
default behaviour here:
--- a/lib/Target/AVR/AVRTargetMachine.cpp
+++ b/lib/Target/AVR/AVRTargetMachine.cpp
@@ -140,6 +140,10 @@ bool AVRPassConfig::addPreEmitPass()
FunctionPass *AVRPassConfig::createTargetRegisterAllocator(bool Optimized)
{
- // Unconditionally use our custom greedy register allocator.
- return createGreedyRegisterAllocator();
+// // Unconditionally use our custom greedy register allocator.
+// return createGreedyRegisterAllocator();
+ if (Optimized)
+ return createGreedyRegisterAllocator();
+ else
+ return createFastRegisterAllocator();
}
2. I have also fixed avr-libc configure.ac to allow avr*clang compilers
too. Current ToT filters out everything that is not in avr*gcc mask.
3. I also have patch applied (with multibytes referencing), I sent you
few days ago.
After I did these three changes, configure script has been finished
successfully. Though during compilation it fails with next error
(assembly file is attached):
/tmp/fclose-3936b6.s: Assembler messages:
/tmp/fclose-3936b6.s:22: Error: can't resolve `0' {*ABS* section} -
`__iob' {*UND* section}
/tmp/fclose-3936b6.s:22: Error: expression too complex
/tmp/fclose-3936b6.s:23: Error: can't resolve `0' {*ABS* section} -
`__iob' {*UND* section}
/tmp/fclose-3936b6.s:23: Error: expression too complex
Note it was for -mmcu=avr5.
P.S.:
Sorry about, I'm not following inline-asm testing strictly. To motivate
myself I need to see not only pedantic unit-test implementation but a
real progress too.
Full invocation log:
stepan@stepan-Inspiron-N5010:/media/stepan/workproj/llvm.project/2-avr/avr-libc/trunk/build.opt/avr/lib/avr5$
/media/stepan/workproj/llvm.project/2-avr/builds/llvm.debug-avr-arm-x86_64.build/Debug+Asserts/bin/avr-clang
-### -DHAVE_CONFIG_H -I. -I../../../../avr-libc/avr/lib/avr5 -I../../..
-I../../../../avr-libc/common -I../../../../avr-libc/include
-I../../../include -Wall -W -Wstrict-prototypes -mmcu=avr5
-D__COMPILING_AVR_LIBC__ -mcall-prologues -Os -MT fclose.o -MD -MP -MF
.deps/fclose.Tpo -c -o fclose.o ../../../../avr-libc/libc/stdio/fclose.c
clang version 3.4 (http://llvm.org/git/clang.git
3aa29df37b140f9c6786b6863a0cac195071b598) (http://llvm.org/git/llvm.git
https://github.com/kaomoneus/avr-llvm.git
ea6c1ebfeac54abd1583e90e518bdf3bab5e1fd5)
Target: avr
Thread model: posix
"/media/stepan/workproj/llvm.project/2-avr/builds/llvm.debug-avr-arm-x86_64.build/Debug+Asserts/bin/clang"
"-cc1" "-triple" "avr" "-S" "-disable-free" "-main-file-name" "fclose.c"
"-mrelocation-model" "static" "-mdisable-fp-elim" "-fmath-errno"
"-mconstructor-aliases" "-target-linker-version" "2.22.90.20120924"
"-coverage-file" "/tmp/fclose-29b051.s" "-resource-dir"
"/media/stepan/workproj/llvm.project/2-avr/builds/llvm.debug-avr-arm-x86_64.build/Debug+Asserts/bin/../lib/clang/3.4"
"-dependency-file" ".deps/fclose.Tpo" "-sys-header-deps" "-MP" "-MT"
"fclose.o" "-D" "HAVE_CONFIG_H" "-D" "__COMPILING_AVR_LIBC__" "-I" "."
"-I" "../../../../avr-libc/avr/lib/avr5" "-I" "../../.." "-I"
"../../../../avr-libc/common" "-I" "../../../../avr-libc/include" "-I"
"../../../include" "-Os" "-Wall" "-W" "-Wstrict-prototypes"
"-fno-dwarf-directory-asm" "-fdebug-compilation-dir"
"/media/stepan/workproj/llvm.project/2-avr/avr-libc/trunk/build.opt/avr/lib/avr5"
"-ferror-limit" "19" "-fmessage-length" "147" "-mstackrealign"
"-fobjc-runtime=gcc" "-fobjc-default-synthesize-properties"
"-fdiagnostics-show-option" "-fcolor-diagnostics" "-vectorize-loops"
"-o" "/tmp/fclose-29b051.s" "-x" "c"
"../../../../avr-libc/libc/stdio/fclose.c"
"/usr/bin/avr-gcc" "-D" "HAVE_CONFIG_H" "-I" "." "-I"
"../../../../avr-libc/avr/lib/avr5" "-I" "../../.." "-I"
"../../../../avr-libc/common" "-I" "../../../../avr-libc/include" "-I"
"../../../include" "-Wall" "-W" "-Wstrict-prototypes" "-mmcu=avr5" "-D"
"__COMPILING_AVR_LIBC__" "-mcall-prologues" "-Os" "-MT" "fclose.o" "-MD"
"-MP" "-MF" ".deps/fclose.Tpo" "-c" "-o" "fclose.o" "-x" "assembler"
"/tmp/fclose-29b051.s"
Borja Ferrer wrote:
> Ok, if this will work with other reg constraints then add some tests to
> cover them aswell.
>
> What errors are you getting exactly with avrlibc? IIRC there's an assert
> in llc that fires when running the greedy regalloc in O0. I dont know
> the reason for this assert and if it can be safely removed, this has to
> be asked in the dev list and also get some possible solutions.
>
>
> 2013/7/17 Stepan Dyatkovskiy <stp...@na... <mailto:stp...@na...>>
>
> Hi Borja.
> It should work for all other constraints as well. I don't like idea
> of fixing clang too. So we can just update
> getRegForInlineAsmConstraint method.
>
> I'd tried to compile avr-libc. Currently clang doesn't want to
> compile everything with -O0. While configure script tests everything
> with -O0. Do you have any ideas how to fix -O0 ?
>
> -Stepan.
>
> Borja Ferrer wrote:
>
> Also, I guess doing only this for r is not enough, you can use
> multibytes with other register constraints.
>
>
> 2013/7/16 Borja Ferrer <bor...@gm...
> <mailto:bor...@gm...>
> <mailto:bor...@gm... <mailto:bor...@gm...>>__>
>
>
> What are you suggesting to do in clang?
>
>
> 2013/7/16 Stepan Dyatkovskiy <stp...@na...
> <mailto:stp...@na...>
> <mailto:stp...@na... <mailto:stp...@na...>>>
>
>
> Back to multibyte referenced.
>
> I've attached patch with this feature + unit-test.
> There is some
> question though.
> Currently, .c code:
> int a;
> void f() {
> asm("instr %A0 %B0": : "r"(a));
> }
> is transformed into IR below:
> define void @f(i16 %a) {
> entry:
> call void asm sideeffect "instr ${0:A} ${0:B}",
> "r"(i16 %a)
> ret void
> }
> To prevent the crash I had to fix
> getRegForInlineAsmConstraint,
> I set it to return DREGS when it see i16 type or
> bigger. Though,
> may be you want to fix clang and replace "r" constraint
> with
> another one.
>
> -Stepan.
>
> Borja Ferrer wrote:
>
> Just remembered after hitting resend that this has
> to be
> done always,
> similarly like in SelectAddr(). However in this
> case it can
> be done
> unconditionally since imm is guaranteed to fit in
> 6bits, so
> even if it
> comes in a constant node of type i64 it's safe to
> truncate
> it to i8.
>
>
> 2013/7/15 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...>>>>
>
> Hi Borja,
>
>
> > + CanHandleRegImmOpt &= ImmNode != 0;
> > + CanHandleRegImmOpt &=
> ImmNode->getAPIntValue().______getZExtValue()
> < 64;
>
>
> You absolutely right here. Thanks.
>
>
> > There's a piece of code i initially removed
> because
> i found it
> redundant:
> > + if (ImmNode->getValueType(0) != MVT::i8)
> > + {
> > + Disp = CurDAG->getTargetConstant(
> > +
> ImmNode->getAPIntValue().______getZExtValue(),
> MVT::i8);
>
> > + }
> > + else
> > + {
> > + Disp = ImmOp;
> > + }
>
> Initially did the same: just "Disp = ImmOp",
> and got
> the crash. I
> don't remember what crash says exactly.
> Currently, I'm
> not able to
> compile anything. What I had found out is that
> Disp (aka
> displacement) should be i8 only. Our back-end just
> don't expect
> other types of displacements.
>
> -Stepan.
>
> Borja Ferrer wrote:
>
> Thanks!
>
> I've been cleaning a bit the code, and fixed a
> possible crash here:
>
> + CanHandleRegImmOpt &= ImmNode != 0;
> + CanHandleRegImmOpt &=
>
> ImmNode->getAPIntValue().______getZExtValue() < 64;
>
>
> Since ImmNode comes from a dyncast, if
> it's NULL
> the second line
> would crash.
>
>
>
> There's a piece of code i initially
> removed because
> i found it
> redundant:
> + if (ImmNode->getValueType(0) !=
> MVT::i8)
> + {
> + Disp = CurDAG->getTargetConstant(
> +
> ImmNode->getAPIntValue().______getZExtValue(),
> MVT::i8);
>
> + }
> + else
> + {
> + Disp = ImmOp;
> + }
>
> but without im getting crashes. Can you
> explain why
> this happens
> and is needed?
>
>
>
> 2013/7/15 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...>>>>>
>
>
> r269
> -Stepan
> Borja Ferrer wrote:
>
> Ohhh this is a lot better,
> atleast fixing
> patches
> iteratively
> brought us
> to a nice solution :)
>
> If this patch passes all tests
> with the
> machine
> verifier enabled
> then
> feel free to commit it. I'll take
> a look
> at the glue
> operands
> after this.
>
>
> 2013/7/12 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...>>>>>>
>
>
> Hello Borja.
> Seems I've made things much
> simpler.
> Can't
> understand why I
> didn't
> it in beginning.
>
> I moved everything into
> SelectInlineAsmMemoryOperand hook.
> #1: We don't need 'for' at
> all. Since it
> implemented in
> hook caller.
> #2: This problem
> disappeared, see #1.
> #4: The same.
> #6: Still not sure about
> glues. I
> just tried to
> keep all
> chains in
> proper order. But you rather
> look at new
> SelectInlineAsmMemoryOperand.
>
>
> > One last thing, when you
> run the
> regression tests,
> enable locally the
> > machine verifier to catch any
> additional errors. To
> enable it
> > unconditionally search
> for the
> command object
> (i think it's
> declared in
> > codegen/passes.cpp as a
> cl::opt)
> and turn it on by
> default. When
> you're
> > doing your own tests you can
> enable it passing
> -verify-machineisntrs or
> > something like that to llc.
> OK.
>
> -Stepan.
>
>
> Borja Ferrer wrote:
>
> Hello Stepan,
>
> 1) Wow, those are really big
> conditions xD.
> You could
> split them
> into
> single conditions and
> use continues:
> if (cond 1) continue;
> if (cond 2) continue;
> etc...
>
> instead of having a huge
> if()
> that is quite
> hard to read.
>
> 2) Yes I thought about that
> aswell, that not all
> constant nodes
> are asm
> flags, but the ARM
> backend does
> this, so
> either they
> have a bug
> there or
> it's safe?
> I prefer if you could
> move the
> big condition
> inside the
> for()
> into the
> top of the loop, it
> makes the
> for() quite
> unreadable.
> And adjust
> there
> the i variable as needed
> for each
> suboperand
> as you
> mentioned
> before.
>
> 4) Why do you say they
> handle
> everything in the
> SelectInlineAsm
> method?
> As far as i can see they
> only
> handle one
> specific case,
> otherwise they
> return NULL and let the
> default
> behaviour do
> the work.
>
> 6) Yes glues are a bit
> tricky,
> dont worry
> about them, I
> will fix
> them
> when this gets commited.
>
> One last thing, when you
> run the
> regression
> tests, enable
> locally the
> machine verifier to
> catch any
> additional
> errors. To
> enable it
> unconditionally search
> for the
> command object
> (i think it's
> declared in
> codegen/passes.cpp as a
> cl::opt)
> and turn it on by
> default. When
> you're
> doing your own tests you can
> enable it passing
> -verify-machineisntrs or
> something like that to llc.
>
>
>
>
>
>
>
>
>
>
>
>
|
|
From: Borja F. <bor...@gm...> - 2013-07-18 14:33:35
|
Heh thanks for those tests. I know you're a bit eager to compile avrlibc :)
but I would like first to get inline asm completed since it's the last big
blocker to get things compilable.
I'm going to try something I thought to see if your multibyte patch works
correctly under certain circumstances.
About your questions:
1) Yes, the main reason is that we use a patched version of the greedy
regalloc, using others will produce wrong code. You can locally use that
patch to get avrlibc compilable but the code produced can be wrong.
2) Can you attach the regenerated configure file so i can try it locally?
The best thing would be if Eric applies it to their svn repo.
I'm aware of this error, I will try to fix it today. Is this the only type
of error you're getting?
2013/7/18 Stepan Dyatkovskiy <stp...@na...>
> Hi Borja,
> Well, ok. I will write several tests for it. Though I think also about
> avr-libc compilation. Today I've made two small fixes in this direction:
>
> 1. Are there any reasons to forbid default FastRA for -O0? Note, currently
> for -O0 it doesn't work at all. So I've temoporary restored default
> behaviour here:
> --- a/lib/Target/AVR/**AVRTargetMachine.cpp
> +++ b/lib/Target/AVR/**AVRTargetMachine.cpp
> @@ -140,6 +140,10 @@ bool AVRPassConfig::addPreEmitPass(**)
>
> FunctionPass *AVRPassConfig::**createTargetRegisterAllocator(**bool
> Optimized)
> {
> - // Unconditionally use our custom greedy register allocator.
> - return createGreedyRegisterAllocator(**);
> +// // Unconditionally use our custom greedy register allocator.
> +// return createGreedyRegisterAllocator(**);
> + if (Optimized)
> + return createGreedyRegisterAllocator(**);
> + else
> + return createFastRegisterAllocator();
> }
>
> 2. I have also fixed avr-libc configure.ac to allow avr*clang compilers
> too. Current ToT filters out everything that is not in avr*gcc mask.
>
> 3. I also have patch applied (with multibytes referencing), I sent you few
> days ago.
>
> After I did these three changes, configure script has been finished
> successfully. Though during compilation it fails with next error (assembly
> file is attached):
>
> /tmp/fclose-3936b6.s: Assembler messages:
> /tmp/fclose-3936b6.s:22: Error: can't resolve `0' {*ABS* section} -
> `__iob' {*UND* section}
> /tmp/fclose-3936b6.s:22: Error: expression too complex
> /tmp/fclose-3936b6.s:23: Error: can't resolve `0' {*ABS* section} -
> `__iob' {*UND* section}
> /tmp/fclose-3936b6.s:23: Error: expression too complex
>
> Note it was for -mmcu=avr5.
>
> P.S.:
> Sorry about, I'm not following inline-asm testing strictly. To motivate
> myself I need to see not only pedantic unit-test implementation but a real
> progress too.
>
> Full invocation log:
>
> stepan@stepan-Inspiron-N5010:/**media/stepan/workproj/llvm.**
> project/2-avr/avr-libc/trunk/**build.opt/avr/lib/avr5$
> /media/stepan/workproj/llvm.**project/2-avr/builds/llvm.**
> debug-avr-arm-x86_64.build/**Debug+Asserts/bin/avr-clang -###
> -DHAVE_CONFIG_H -I. -I../../../../avr-libc/avr/**lib/avr5 -I../../..
> -I../../../../avr-libc/common -I../../../../avr-libc/include
> -I../../../include -Wall -W -Wstrict-prototypes -mmcu=avr5
> -D__COMPILING_AVR_LIBC__ -mcall-prologues -Os -MT fclose.o -MD -MP -MF
> .deps/fclose.Tpo -c -o fclose.o ../../../../avr-libc/libc/**stdio/fclose.c
> clang version 3.4 (http://llvm.org/git/clang.git3aa29df37b140f9c6786b6863a0cac
> **195071b598) (http://llvm.org/git/llvm.git https://github.com/kaomoneus/*
> *avr-llvm.git <https://github.com/kaomoneus/avr-llvm.git>ea6c1ebfeac54abd1583e90e518bdf
> **3bab5e1fd5)
> Target: avr
> Thread model: posix
>
> "/media/stepan/workproj/llvm.**project/2-avr/builds/llvm.**
> debug-avr-arm-x86_64.build/**Debug+Asserts/bin/clang" "-cc1" "-triple"
> "avr" "-S" "-disable-free" "-main-file-name" "fclose.c"
> "-mrelocation-model" "static" "-mdisable-fp-elim" "-fmath-errno"
> "-mconstructor-aliases" "-target-linker-version" "2.22.90.20120924"
> "-coverage-file" "/tmp/fclose-29b051.s" "-resource-dir"
> "/media/stepan/workproj/llvm.**project/2-avr/builds/llvm.**
> debug-avr-arm-x86_64.build/**Debug+Asserts/bin/../lib/**clang/3.4"
> "-dependency-file" ".deps/fclose.Tpo" "-sys-header-deps" "-MP" "-MT"
> "fclose.o" "-D" "HAVE_CONFIG_H" "-D" "__COMPILING_AVR_LIBC__" "-I" "." "-I"
> "../../../../avr-libc/avr/lib/**avr5" "-I" "../../.." "-I"
> "../../../../avr-libc/common" "-I" "../../../../avr-libc/include" "-I"
> "../../../include" "-Os" "-Wall" "-W" "-Wstrict-prototypes"
> "-fno-dwarf-directory-asm" "-fdebug-compilation-dir"
> "/media/stepan/workproj/llvm.**project/2-avr/avr-libc/trunk/**build.opt/avr/lib/avr5"
> "-ferror-limit" "19" "-fmessage-length" "147" "-mstackrealign"
> "-fobjc-runtime=gcc" "-fobjc-default-synthesize-**properties"
> "-fdiagnostics-show-option" "-fcolor-diagnostics" "-vectorize-loops" "-o"
> "/tmp/fclose-29b051.s" "-x" "c" "../../../../avr-libc/libc/**
> stdio/fclose.c"
> "/usr/bin/avr-gcc" "-D" "HAVE_CONFIG_H" "-I" "." "-I"
> "../../../../avr-libc/avr/lib/**avr5" "-I" "../../.." "-I"
> "../../../../avr-libc/common" "-I" "../../../../avr-libc/include" "-I"
> "../../../include" "-Wall" "-W" "-Wstrict-prototypes" "-mmcu=avr5" "-D"
> "__COMPILING_AVR_LIBC__" "-mcall-prologues" "-Os" "-MT" "fclose.o" "-MD"
> "-MP" "-MF" ".deps/fclose.Tpo" "-c" "-o" "fclose.o" "-x" "assembler"
> "/tmp/fclose-29b051.s"
>
> Borja Ferrer wrote:
>
>> Ok, if this will work with other reg constraints then add some tests to
>> cover them aswell.
>>
>> What errors are you getting exactly with avrlibc? IIRC there's an assert
>> in llc that fires when running the greedy regalloc in O0. I dont know
>> the reason for this assert and if it can be safely removed, this has to
>> be asked in the dev list and also get some possible solutions.
>>
>>
>> 2013/7/17 Stepan Dyatkovskiy <stp...@na... <mailto:stp...@na...
>> >>
>>
>>
>> Hi Borja.
>> It should work for all other constraints as well. I don't like idea
>> of fixing clang too. So we can just update
>> getRegForInlineAsmConstraint method.
>>
>> I'd tried to compile avr-libc. Currently clang doesn't want to
>> compile everything with -O0. While configure script tests everything
>> with -O0. Do you have any ideas how to fix -O0 ?
>>
>> -Stepan.
>>
>> Borja Ferrer wrote:
>>
>> Also, I guess doing only this for r is not enough, you can use
>> multibytes with other register constraints.
>>
>>
>> 2013/7/16 Borja Ferrer <bor...@gm...
>> <mailto:bor...@gm...>
>> <mailto:bor...@gm... <mailto:bor...@gm...>**
>> >__>
>>
>>
>>
>> What are you suggesting to do in clang?
>>
>>
>> 2013/7/16 Stepan Dyatkovskiy <stp...@na...
>> <mailto:stp...@na...>
>> <mailto:stp...@na... <mailto:stp...@na...>>>
>>
>>
>>
>> Back to multibyte referenced.
>>
>> I've attached patch with this feature + unit-test.
>> There is some
>> question though.
>> Currently, .c code:
>> int a;
>> void f() {
>> asm("instr %A0 %B0": : "r"(a));
>> }
>> is transformed into IR below:
>> define void @f(i16 %a) {
>> entry:
>> call void asm sideeffect "instr ${0:A} ${0:B}",
>> "r"(i16 %a)
>> ret void
>> }
>> To prevent the crash I had to fix
>> getRegForInlineAsmConstraint,
>> I set it to return DREGS when it see i16 type or
>> bigger. Though,
>> may be you want to fix clang and replace "r" constraint
>> with
>> another one.
>>
>> -Stepan.
>>
>> Borja Ferrer wrote:
>>
>> Just remembered after hitting resend that this has
>> to be
>> done always,
>> similarly like in SelectAddr(). However in this
>> case it can
>> be done
>> unconditionally since imm is guaranteed to fit in
>> 6bits, so
>> even if it
>> comes in a constant node of type i64 it's safe to
>> truncate
>> it to i8.
>>
>>
>> 2013/7/15 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...
>> >>>>
>>
>> Hi Borja,
>>
>>
>> > + CanHandleRegImmOpt &= ImmNode != 0;
>> > + CanHandleRegImmOpt &=
>> ImmNode->getAPIntValue()._____**_getZExtValue()
>>
>> < 64;
>>
>>
>> You absolutely right here. Thanks.
>>
>>
>> > There's a piece of code i initially removed
>> because
>> i found it
>> redundant:
>> > + if (ImmNode->getValueType(0) !=
>> MVT::i8)
>> > + {
>> > + Disp = CurDAG->getTargetConstant(
>> > +
>> ImmNode->getAPIntValue()._____**_getZExtValue(),
>>
>> MVT::i8);
>>
>> > + }
>> > + else
>> > + {
>> > + Disp = ImmOp;
>> > + }
>>
>> Initially did the same: just "Disp = ImmOp",
>> and got
>> the crash. I
>> don't remember what crash says exactly.
>> Currently, I'm
>> not able to
>> compile anything. What I had found out is that
>> Disp (aka
>> displacement) should be i8 only. Our back-end
>> just
>> don't expect
>> other types of displacements.
>>
>> -Stepan.
>>
>> Borja Ferrer wrote:
>>
>> Thanks!
>>
>> I've been cleaning a bit the code, and
>> fixed a
>> possible crash here:
>>
>> + CanHandleRegImmOpt &= ImmNode != 0;
>> + CanHandleRegImmOpt &=
>>
>> ImmNode->getAPIntValue()._____**_getZExtValue() < 64;
>>
>>
>>
>> Since ImmNode comes from a dyncast, if
>> it's NULL
>> the second line
>> would crash.
>>
>>
>>
>> There's a piece of code i initially
>> removed because
>> i found it
>> redundant:
>> + if (ImmNode->getValueType(0) !=
>> MVT::i8)
>> + {
>> + Disp = CurDAG->getTargetConstant(
>> +
>> ImmNode->getAPIntValue()._____**_getZExtValue(),
>>
>> MVT::i8);
>>
>> + }
>> + else
>> + {
>> + Disp = ImmOp;
>> + }
>>
>> but without im getting crashes. Can you
>> explain why
>> this happens
>> and is needed?
>>
>>
>>
>> 2013/7/15 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...>>>>>
>>
>>
>> r269
>> -Stepan
>> Borja Ferrer wrote:
>>
>> Ohhh this is a lot better,
>> atleast fixing
>> patches
>> iteratively
>> brought us
>> to a nice solution :)
>>
>> If this patch passes all tests
>> with the
>> machine
>> verifier enabled
>> then
>> feel free to commit it. I'll take
>> a look
>> at the glue
>> operands
>> after this.
>>
>>
>> 2013/7/12 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...>>>>>**>
>>
>>
>> Hello Borja.
>> Seems I've made things much
>> simpler.
>> Can't
>> understand why I
>> didn't
>> it in beginning.
>>
>> I moved everything into
>> SelectInlineAsmMemoryOperand hook.
>> #1: We don't need 'for' at
>> all. Since it
>> implemented in
>> hook caller.
>> #2: This problem
>> disappeared, see #1.
>> #4: The same.
>> #6: Still not sure about
>> glues. I
>> just tried to
>> keep all
>> chains in
>> proper order. But you rather
>> look at new
>> SelectInlineAsmMemoryOperand.
>>
>>
>> > One last thing, when you
>> run the
>> regression tests,
>> enable locally the
>> > machine verifier to catch
>> any
>> additional errors. To
>> enable it
>> > unconditionally search
>> for the
>> command object
>> (i think it's
>> declared in
>> > codegen/passes.cpp as a
>> cl::opt)
>> and turn it on by
>> default. When
>> you're
>> > doing your own tests you
>> can
>> enable it passing
>> -verify-machineisntrs or
>> > something like that to llc.
>> OK.
>>
>> -Stepan.
>>
>>
>> Borja Ferrer wrote:
>>
>> Hello Stepan,
>>
>> 1) Wow, those are really
>> big
>> conditions xD.
>> You could
>> split them
>> into
>> single conditions and
>> use continues:
>> if (cond 1) continue;
>> if (cond 2) continue;
>> etc...
>>
>> instead of having a huge
>> if()
>> that is quite
>> hard to read.
>>
>> 2) Yes I thought about
>> that
>> aswell, that not all
>> constant nodes
>> are asm
>> flags, but the ARM
>> backend does
>> this, so
>> either they
>> have a bug
>> there or
>> it's safe?
>> I prefer if you could
>> move the
>> big condition
>> inside the
>> for()
>> into the
>> top of the loop, it
>> makes the
>> for() quite
>> unreadable.
>> And adjust
>> there
>> the i variable as needed
>> for each
>> suboperand
>> as you
>> mentioned
>> before.
>>
>> 4) Why do you say they
>> handle
>> everything in the
>> SelectInlineAsm
>> method?
>> As far as i can see they
>> only
>> handle one
>> specific case,
>> otherwise they
>> return NULL and let the
>> default
>> behaviour do
>> the work.
>>
>> 6) Yes glues are a bit
>> tricky,
>> dont worry
>> about them, I
>> will fix
>> them
>> when this gets commited.
>>
>> One last thing, when you
>> run the
>> regression
>> tests, enable
>> locally the
>> machine verifier to
>> catch any
>> additional
>> errors. To
>> enable it
>> unconditionally search
>> for the
>> command object
>> (i think it's
>> declared in
>> codegen/passes.cpp as a
>> cl::opt)
>> and turn it on by
>> default. When
>> you're
>> doing your own tests you
>> can
>> enable it passing
>> -verify-machineisntrs or
>> something like that to
>> llc.
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>
|
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X