Menu
▾
▴
Re: [avr-llvm-devel] Inline assembly. Mostly just a stub.
|
From: Borja F. <bor...@gm...> - 2013-06-17 23:38:24
|
Sure Stepan, go ahead. I can do any post commit cleanups afterwards.
2013/6/17 John Myers <ato...@gm...>
> 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>>>
>>
>>
>>
>>
>>
>>
>>
>>
>
|
