Menu
▾
▴
Re: [avr-llvm-devel] Inline assembly. Mostly just a stub.
|
From: Borja F. <bor...@gm...> - 2013-06-17 18:50:13
|
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>>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>
|
