From: Stepan D. <stp...@na...> - 2013-06-20 13:02:57
|
Hello Borja, > 1) for the M constraint you expand the type to MVT::i16 in case the > value is negative. I dont understand the need to do this. By default AsmWriter treats constants as signed values. So "i8 255" would be printed as "i8 -1". But I'm not sure that avr-as would interpret this as "*i8* -1" (I even definitely sure it fails some code). So I'v made it i16; doing that, we can be sure that values in range 0..255 would be printed as unsigned. May be it is kind of hack, though on the first stage I'd just make things working... > 2) did you manage to understand how the G constraint works? I see you > return a 0 MVT::i8 constant, but floats are 32 bits so how does this > really work. IMHO, kind of stupid constraint. avr-gcc doesn't eat .c inline asm string like below: 'asm("instr %0"::"G"(0.1) );' avr-gcc exits with error: impossible constraint in ‘asm’ while 'asm("instr %0"::"G"(0.0) )' works fine. 'asm("instr %0"::"G"() );' doesn't work either. So '"G"(0.0)' is the only correct case. I set it to i8, since we just need convert it to zero. I suppose for avr-as it doesn't matter which kind of zero would be presented (0, 0x0 or 0.000). E.g. avr-gcc prints 0x0. -Stepan. P.S.: Just as playground code: // file: test.c // cmd: avr-gcc test.c -S -o - void f() { int a,b,c,d; asm("instr %0"::"G"(0.0) ); } > > > > > 2013/6/18 Borja Ferrer <bor...@gm... > <mailto:bor...@gm...>> > > Sure Stepan, go ahead. I can do any post commit cleanups afterwards. > > > 2013/6/17 John Myers <ato...@gm... > <mailto:ato...@gm...>> > > You should have commit access. > > > On Mon, Jun 17, 2013 at 1:35 PM, Stepan Dyatkovskiy > <stp...@na... <mailto:stp...@na...>> wrote: > > Hi Borja, > 2.1: > You're right. Will return C_Register. > About Q, I'd want to learn a bit more before it would be > implemented. Perhaps there is nothing special, but I'd want > to check. > 2.2. Yes, we can return cw_constant. I had used ARM template > :-) They just had set it as cw_other :-) > 3. avr-llvm has appeared in "My Projects" in source forge > menu. Does it mean I have permissions already? Can I do test > commit (one more \n in README)? > 4. I propose to commit everything (with fixes you proposed) > to avoid growing snow ball patch ;-) > -- > Truly yours, > Stepan Dyatkovskiy > 17.06.2013, 22:50, "Borja Ferrer" <bor...@gm... > <mailto:bor...@gm...>>: >> Ok thanks for the fixes, more things: >> >> 2.1) AVRTargetLowering::getConstraintType: Looking at >> other backends, I think the x, y, z, t constraints should >> return C_Register instead of C_RegisterClass since these >> are used for specific regs inside a regclass. Probably the >> same for the q one. What do you think? >> >> 2.2) Ok, so you've given the largest weight to the bigger >> regclasses d, r, l and then for the other reg constraints >> the smaller weight, I agree then. >> Now, for the constant constraints you should return >> CW_Constant instead, other backends first check that the >> imm val is in range and if it is then set it to that >> value, this is taken from the x86 one: >> case 'K': >> if (ConstantInt *C = >> dyn_cast<ConstantInt>(CallOperandVal)) { >> if ((C->getSExtValue() >= -0x80) && >> (C->getSExtValue() <= 0x7f)) >> weight = CW_Constant; >> } >> break; >> Finally for Q we should return CW_Memory no? >> >> 2.3) In getRegForInlineAsmConstraint, if this inline asm >> stuff is run after type legalization you could turn that >> type checking at the top of the function into an assert() >> comment: Upper register r16..r32. <<-- typo registerS and r32 >> >> John or Eric, please give Stepan commit permissions. >> >> >> 2013/6/17 Stepan Dyatkovskiy <stp...@na... >> <mailto:stp...@na...>> >> >> Hello Borja, >> >> >> ok here we go: >> >> 1) in AVRRegisterInfo.td: im ok with the lGPR8 >> regclass since we dont >> have it yet and i guess it's needed for the l >> constraint, but rename it >> to GPR8lo. The hGPR8 regclass comment says lower >> registers, so typo >> there, but anyways can't you use the LD8 regclass? >> Rename simplehGPR8 to >> LD8lo, we'll need it in the future for some MUL >> instructions. >> >> Everything here were fixed as you mentioned. >> >> >> >> 2) in AVRISelLowering.cpp: for the t, x, y, z >> shouldn't we return >> C_Register? Probably for b aswell. >> 2.1) in getSingleConstraintMatchWeight can you >> clarify me what's the >> diff between CW_SpecificReg and CW_Register. >> Shouldn't the G constraint >> be a Constant? Same for the other constraints? >> >> All this stuff with weights is due to support of >> multiple constraints. For some operands you may set >> *the set* of constraints, e.g. 'mr': get either memory >> of register. In that case we're use weights. What >> should we select for 'bx' constraint for example? One >> of y,z or x? So, currently llvm gets register first >> from class with bigger weight. Since CW_SpecificReg < >> CW_Register it will select one of y,z. >> >> For more information see implementation of >> "TargetLowering::__getMultipleConstraintMatchWeig__ht" >> and "TargetLowering::__ParseConstraints". >> >> 'G' shouldn't be CW_Register, of course, that was my >> typo. I've fixed it, now it as all other constants >> just a CW_Default. >> >> >> 2.2) in getRegForInlineAsmConstraint fixup the >> regclasses per point (1). >> for w you could use the IWREGS regclass, the docs >> say they are regpairs, >> not 8 bit regs as you declared in the .td file. >> >> That was also fixed. >> >> -Stepan. >> >> >> The rest looks great. >> >> >> 2013/6/14 Borja Ferrer <bor...@gm... >> <mailto:bor...@gm...> >> <mailto:bor...@gm... >> <mailto:bor...@gm...>>__> >> >> >> Going to review the patch now. >> >> No, there is no test for allocation order, I >> don't know a good way >> of testing that. If you want you can replace >> the register list by >> several sequences. >> >> >> 2013/6/14 Stepan Dyatkovskiy >> <stp...@na... <mailto:stp...@na...> >> <mailto:stp...@na... >> <mailto:stp...@na...>>> >> >> >> Hi Borja, >> All constraints are supported now. >> The new patch is attached. >> >> -- Added support for all documented >> constraints. >> -- Fixed tests as you mentioned. >> >> Relative to GPR8, >> Do we have some tests that check >> allocation order? If so, all of >> them were passed :-) >> >> Though I can use the explicit enumeration >> way you're currently >> using... >> >> -Stepan. >> >> Borja Ferrer wrote: >> >> Ok, there are some small style issues >> but i will fix them >> after you can >> commit (braces in >> AVRAsmPrinter::____PrintAsmOperand), >> >> afterwards you may >> check them for future reference. >> >> About the GPR8 replacement, I'm not >> sure now, but will it >> change the >> register allocation order using the >> "sequence" set >> instruction? This was >> changed some time ago so I dont >> remember what was the new...@na... >> <mailto:stp...@na... >> <mailto:stp...@na...>>>>> >> >> >> >> Hi all, >> I'll use this >> reference for >> implementation, right? >> http://savannah.nongnu.org/______download/avr-libc/avr-libc-______user-manual-1.8.0.pdf.bz2 >> <http://savannah.nongnu.org/____download/avr-libc/avr-libc-____user-manual-1.8.0.pdf.bz2> >> >> <http://savannah.nongnu.org/____download/avr-libc/avr-libc-____user-manual-1.8.0.pdf.bz2 >> <http://savannah.nongnu.org/__download/avr-libc/avr-libc-__user-manual-1.8.0.pdf.bz2>> >> >> >> >> >> <http://savannah.nongnu.org/____download/avr-libc/avr-libc-____user-manual-1.8.0.pdf.bz2 >> <http://savannah.nongnu.org/__download/avr-libc/avr-libc-__user-manual-1.8.0.pdf.bz2> >> >> <http://savannah.nongnu.org/__download/avr-libc/avr-libc-__user-manual-1.8.0.pdf.bz2 >> <http://savannah.nongnu.org/download/avr-libc/avr-libc-user-manual-1.8.0.pdf.bz2>>> >> >> -Stepan. >> >> Stepan Dyatkovskiy >> wrote: >> > Ops. Forget to >> apply patch itself... >> > >> > -Stepan. >> > >> > Stepan >> Dyatkovskiy wrote: >> >> Hi all. That's a >> Thursday patch with >> inline asm. >> Currently >> the only >> >> constraint is >> supported: register ('r'). >> >> >> >> -Stepan. >> >> >> >> >> >> >> ------------------------------______--------------------------__--__--__------------------ >> >> >> >> >> >> This SF.net >> email is sponsored by Windows: >> >> >> >> Build for >> Windows Store. >> >> >> >> >> http://p.sf.net/sfu/windows-______dev2dev >> <http://p.sf.net/sfu/windows-____dev2dev> >> >> <http://p.sf.net/sfu/windows-____dev2dev >> <http://p.sf.net/sfu/windows-__dev2dev>> >> >> <http://p.sf.net/sfu/windows-____dev2dev >> <http://p.sf.net/sfu/windows-__dev2dev> >> <http://p.sf.net/sfu/windows-__dev2dev >> <http://p.sf.net/sfu/windows-dev2dev>>> >> >> >> >> _____________________________________________________ >> >> avr-llvm-devel >> mailing list >> >> >> avr-llvm-devel@lists.__ >> <mailto:avr-llvm-devel@lists.__>sourcef____orge.net <http://sourcef__orge.net/> >> <http://sourceforge.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...>>> >> >> <mailto:avr-llvm-devel@lists >> <mailto:avr-llvm-devel@lists>. >> <mailto:avr-llvm-devel@lists >> <mailto:avr-llvm-devel@lists>.>______sourceforge.net >> <http://sourceforge.net/> >> <http://sourceforge.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...>>>> >> >> >> >> 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. >> <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>>> >> >> >> > >> > >> > >> > >> >> >> >> ------------------------------______--------------------------__--__--__------------------ >> >> >> > This SF.net email >> is sponsored by Windows: >> > >> > Build for Windows >> Store. >> > >> > >> http://p.sf.net/sfu/windows-______dev2dev >> <http://p.sf.net/sfu/windows-____dev2dev> >> >> <http://p.sf.net/sfu/windows-____dev2dev >> <http://p.sf.net/sfu/windows-__dev2dev>> >> >> <http://p.sf.net/sfu/windows-____dev2dev >> <http://p.sf.net/sfu/windows-__dev2dev> >> <http://p.sf.net/sfu/windows-__dev2dev >> <http://p.sf.net/sfu/windows-dev2dev>>> >> > >> > >> > >> > >> >> _____________________________________________________ >> > avr-llvm-devel >> mailing list >> > >> avr-llvm-devel@lists.__ >> <mailto:avr-llvm-devel@lists.__>sourcef____orge.net <http://sourcef__orge.net/> >> <http://sourceforge.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...>>> >> >> <mailto:avr-llvm-devel@lists >> <mailto:avr-llvm-devel@lists>. >> <mailto:avr-llvm-devel@lists >> <mailto:avr-llvm-devel@lists>.>______sourceforge.net >> <http://sourceforge.net/> >> <http://sourceforge.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...>>>> >> >> > >> 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. >> <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>>> >> > >> >> >> >> >> >> ------------------------------______--------------------------__--__--__------------------ >> >> >> This SF.net email is >> sponsored by Windows: >> >> Build for Windows Store. >> http://p.sf.net/sfu/windows-______dev2dev >> <http://p.sf.net/sfu/windows-____dev2dev> >> >> <http://p.sf.net/sfu/windows-____dev2dev >> <http://p.sf.net/sfu/windows-__dev2dev>> >> >> <http://p.sf.net/sfu/windows-____dev2dev >> <http://p.sf.net/sfu/windows-__dev2dev> >> <http://p.sf.net/sfu/windows-__dev2dev >> <http://p.sf.net/sfu/windows-dev2dev>>> >> >> >> _____________________________________________________ >> avr-llvm-devel >> mailing list >> avr-llvm-devel@lists.__ >> <mailto:%C2%A0avr-llvm-devel@lists.__>sourcef____orge.net >> <http://sourcef__orge.net/> >> <http://sourceforge.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...>>> >> >> <mailto:avr-llvm-devel@lists >> <mailto:avr-llvm-devel@lists>. >> <mailto:avr-llvm-devel@lists >> <mailto:avr-llvm-devel@lists>.>______sourceforge.net >> <http://sourceforge.net/> >> <http://sourceforge.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...>>>> >> 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. >> <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>>> >> >> >> >> >> >> >> > > > |