Re: [avr-llvm-devel] Inline assembly.

[Stepan D. <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>
>                   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>
>
>
>