[llvm] r298652 - Move spill size and alignment info from MC to TargetRegisterInfo
Quentin Colombet via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 24 12:44:29 PDT 2017
Hi Krzystof,
> On Mar 24, 2017, at 12:40 PM, Krzysztof Parzyszek <kparzysz at codeaurora.org> wrote:
>
> Hi Quentin,
> This has been discussed a while back, and you were specifically invited:
> https://reviews.llvm.org/D24631 <https://reviews.llvm.org/D24631>
Yeah, I missed it.
>
> I did originally plan to put all this information in the MC layer, and changed this at the suggestion from Matthias.
>
> I reverted it for now, but I expect that we find some agreement here.
Of course! Like I said, I believe the patch you suggested looks good modulo keeping the MC getSize piece and renaming the TRI getSpillSize.
For now the MC getSize would contain the same thing as the TRI getSpillSize, but I expect your next patches to change that.
Cheers,
-Quentin
>
> -Krzysztof
>
>
> On 3/23/2017 8:25 PM, Quentin Colombet wrote:
>> Hi Krzysztof,
>>
>> In an out of tree target, we do access the size of in the MC layer. Thus this change breaks us.
>>
>> I agree with the intent of the patch, but the MC layer should still have access to the size of the register (which admittedly we conflict with the spill size for now).
>>
>> Could you revert please?
>>
>> Also, given the impact of that change (changing the layer of some information), I believe it would have make sense to discuss it on the dev list or put more reviewers.
>>
>> Going forward, I would suggest to add a getSpillSize in TRI and keep getSize in MC.
>>
>> Cheers,
>> -Quentin
>>> On Mar 23, 2017, at 3:32 PM, Krzysztof Parzyszek via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>>
>>> Author: kparzysz
>>> Date: Thu Mar 23 17:32:22 2017
>>> New Revision: 298652
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=298652&view=rev
>>> Log:
>>> Move spill size and alignment info from MC to TargetRegisterInfo
>>>
>>> This is another step towards implementing register classes with
>>> parametrized register/spill sizes.
>>>
>>> Differential Revision: https://reviews.llvm.org/D31299
>>>
>>> Modified:
>>> llvm/trunk/include/llvm/MC/MCRegisterInfo.h
>>> llvm/trunk/include/llvm/Target/TargetRegisterInfo.h
>>> llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp
>>>
>>> Modified: llvm/trunk/include/llvm/MC/MCRegisterInfo.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCRegisterInfo.h?rev=298652&r1=298651&r2=298652&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/MC/MCRegisterInfo.h (original)
>>> +++ llvm/trunk/include/llvm/MC/MCRegisterInfo.h Thu Mar 23 17:32:22 2017
>>> @@ -41,7 +41,6 @@ public:
>>> const uint16_t RegsSize;
>>> const uint16_t RegSetSize;
>>> const uint16_t ID;
>>> - const uint16_t RegSize, Alignment; // Size & Alignment of register in bytes
>>> const int8_t CopyCost;
>>> const bool Allocatable;
>>>
>>> @@ -80,14 +79,6 @@ public:
>>> return contains(Reg1) && contains(Reg2);
>>> }
>>>
>>> - /// getSize - Return the size of the register in bytes, which is also the size
>>> - /// of a stack slot allocated to hold a spilled copy of this register.
>>> - unsigned getSize() const { return RegSize; }
>>> -
>>> - /// getAlignment - Return the minimum required alignment for a register of
>>> - /// this class.
>>> - unsigned getAlignment() const { return Alignment; }
>>> -
>>> /// getCopyCost - Return the cost of copying a value between two registers in
>>> /// this class. A negative number means the register class is very expensive
>>> /// to copy e.g. status flag register classes.
>>>
>>> Modified: llvm/trunk/include/llvm/Target/TargetRegisterInfo.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetRegisterInfo.h?rev=298652&r1=298651&r2=298652&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/Target/TargetRegisterInfo.h (original)
>>> +++ llvm/trunk/include/llvm/Target/TargetRegisterInfo.h Thu Mar 23 17:32:22 2017
>>> @@ -45,6 +45,7 @@ public:
>>>
>>> // Instance variables filled by tablegen, do not use!
>>> const MCRegisterClass *MC;
>>> + const uint16_t SpillSize, SpillAlignment;
>>> const vt_iterator VTs;
>>> const uint32_t *SubClassMask;
>>> const uint16_t *SuperRegIndices;
>>> @@ -94,10 +95,10 @@ public:
>>>
>>> /// Return the size of the register in bytes, which is also the size
>>> /// of a stack slot allocated to hold a spilled copy of this register.
>>> - unsigned getSize() const { return MC->getSize(); }
>>> + unsigned getSize() const { return SpillSize; }
>>>
>>> /// Return the minimum required alignment for a register of this class.
>>> - unsigned getAlignment() const { return MC->getAlignment(); }
>>> + unsigned getAlignment() const { return SpillAlignment; }
>>>
>>> /// Return the cost of copying a value between two registers in this class.
>>> /// A negative number means the register class is very expensive
>>>
>>> Modified: llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp?rev=298652&r1=298651&r2=298652&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp (original)
>>> +++ llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp Thu Mar 23 17:32:22 2017
>>> @@ -1025,16 +1025,12 @@ RegisterInfoEmitter::runMCDesc(raw_ostre
>>> for (const auto &RC : RegisterClasses) {
>>> // Asserts to make sure values will fit in table assuming types from
>>> // MCRegisterInfo.h
>>> - assert((RC.SpillSize/8) <= 0xffff && "SpillSize too large.");
>>> - assert((RC.SpillAlignment/8) <= 0xffff && "SpillAlignment too large.");
>>> assert(RC.CopyCost >= -128 && RC.CopyCost <= 127 && "Copy cost too large.");
>>>
>>> OS << " { " << RC.getName() << ", " << RC.getName() << "Bits, "
>>> << RegClassStrings.get(RC.getName()) << ", "
>>> << RC.getOrder().size() << ", sizeof(" << RC.getName() << "Bits), "
>>> << RC.getQualifiedName() + "RegClassID" << ", "
>>> - << RC.SpillSize/8 << ", "
>>> - << RC.SpillAlignment/8 << ", "
>>> << RC.CopyCost << ", "
>>> << ( RC.Allocatable ? "true" : "false" ) << " },\n";
>>> }
>>> @@ -1316,9 +1312,13 @@ RegisterInfoEmitter::runTargetDesc(raw_o
>>> << " { // Register class instances\n";
>>>
>>> for (const auto &RC : RegisterClasses) {
>>> + assert(isUInt<16>(RC.SpillSize/8) && "SpillSize too large.");
>>> + assert(isUInt<16>(RC.SpillAlignment/8) && "SpillAlignment too large.");
>>> OS << " extern const TargetRegisterClass " << RC.getName()
>>> << "RegClass = {\n " << '&' << Target.getName()
>>> << "MCRegisterClasses[" << RC.getName() << "RegClassID],\n "
>>> + << RC.SpillSize/8 << ", /* SpillSize */\n "
>>> + << RC.SpillAlignment/8 << ", /* SpillAlignment */\n "
>>> << "VTLists + " << VTSeqs.get(RC.VTs) << ",\n " << RC.getName()
>>> << "SubClassMask,\n SuperRegIdxSeqs + "
>>> << SuperRegIdxSeqs.get(SuperRegIdxLists[RC.EnumValue]) << ",\n ";
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170324/a8b1931c/attachment.html>
More information about the llvm-commits
mailing list