Menu
▾
▴
Re: [avr-llvm-devel] Inline assembly. Mostly just a stub.
|
From: Borja F. <bor...@gm...> - 2013-06-20 00:10:05
|
Stepan thanks for the commit!
I've done some post commit style fixes and fix a possible crash with a
dyn_cast.
I have two further questions after reviewing your code more thoroughly:
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.
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.
2013/6/18 Borja Ferrer <bor...@gm...>
> 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>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>
>
|
