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