From: John M. <ato...@gm...> - 2013-06-17 20:39:31
|
You should have commit access. On Mon, Jun 17, 2013 at 1:35 PM, Stepan Dyatkovskiy <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...>: > > 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...> > > 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...>**> > > > 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...>> > > > 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 > behaviour. > In the test cases please add a CHECK line for the function > name, check > the other test cases in the folder to see what i mean. > You may need to add inline asm support in clang aswell, > there are some > methods that need to be implemented there inside the AVR > classes, > probably for error reporting stuff. > > 1) I dont know of any compatible projects, sorry. I've > tested big C > programs like a floating point emulation library. > 2) for now only the tests there. > 3) John please give Stepan commit access to SVN. > > Heh Eric, that's ok, for inline asm there's no room for > optimizations > but for other compiler features sure :) > > > > > 2013/6/13 Stepan Dyatkovskiy <stp...@na... > <mailto:stp...@na...> <mailto:stp...@na... > <mailto:stp...@na...>>> > > Hi Borja, > This is today patch. > 1. Added and tested support for register constraints > (a,b,d,e,q,r,t,w,x,y,z,l). > 2. Refactored GPR8 registers class, added new registers > classes: > lGPR8, hGPR8, simplehGPR8 and specialUpperGPR8. > 3. Fixed code-style. > > P.S.: In case you decided to give me commit access: > 1. Do you have some AVR projects that are compilable > with avr-llvm? > I mean kind of llvm's test-suite replacement. > 2. What ought to be tested before commit, except the > "test/CodeGen/AVR" tests? > 3. My SourceForge account name is: "kaomoneus". > > -Stepan. > > Borja Ferrer wrote: > > The patch looks good, some comments about coding > style. We use a > different standard compared to llvm's, most > important things are: > 1) braces go into new lines. > 2) always place braces even if they're for a 1 line > statement > > this: > + std::pair<unsigned, const TargetRegisterClass*> > + getRegForInlineAsmConstraint(_**___const > > std::string > > &Constraint, > + EVT VT) const; > + > should be: > std::pair<unsigned, const TargetRegisterClass *> > << space > between > name and * > getRegForInlineAsmConstraint(_**___const std::string > > &Constraint, > EVT VT) const; > > this: > + ConstraintWeight getSingleConstraintMatchWeight** > ____( > > > + AsmOperandInfo &info, const char *constraint) > const; > should be: > ConstraintWeight > getSingleConstraintMatchWeight**____(AsmOperandInfo > > &info, > > > const char > *constraint) > const; > > > if there are any other small details i can fix them > post commit. > > > > 2013/6/13 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...>**>__>__> > > > > > Nice Stepan, I will review your patch later > on. You should > be given > commit access to this svn repo until we > finally decide > where to move > on all of the code. > > > 2013/6/13 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 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.__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...> > >> > <mailto:avr-llvm-devel@lists. > <mailto:avr-llvm-devel@lists.>**____sourceforge.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>>> > >> > > > > > > > > > > > ------------------------------**____-------------------------- > **--__--__------------------ > > > > 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.__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...> > >> > <mailto:avr-llvm-devel@lists. > <mailto:avr-llvm-devel@lists.>**____sourceforge.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>>> > > > > > > > ------------------------------**____-------------------------- > **--__--__------------------ > > > 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.__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...> > >> > <mailto:avr-llvm-devel@lists. > <mailto:avr-llvm-devel@lists.>**____sourceforge.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>>> > > > > > > > > |