Menu
▾
▴
Re: [avr-llvm-devel] Inline assembly. Mostly just a stub.
|
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>>>
>
>
>
>
>
>
>
>
|
