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