[llvm-dev] The Trouble with Triples

Tue Sep 15 09:30:42 PDT 2015

>
> I was writing a rather longer email with an actual plan for you, but as
> you've committed I'll send it briefly.
>
> While you've done a lot of (legitimate) complaining about mips I don't see
> how this is an effective long term strategy for how we communicate
> information within llvm. This mostly duplicates a lot of data that we have
> sitting around and puts it into an interface. Why aren't you fixing the
> interfaces rather than duplicating the data? It's a complicated interface
> right now and a lot of work to untangle it, but that's what's needed.
>
> The largest part of the problem for the mips port is that the assembler
> needs information that only the TargetMachine has in any useful way. The
> better idea is to:
>
> a) get rid of the is-a interface for MCSubtargetInfo/TargetSubtargetInfo
> and replace that with a has-a interface so that you can look up information
> directly and only IR level compilation information remains at the
> TargetSubtargetInfo interface, while
>
> b) probably adding a TargetMachine equivalent at the MC level. This could
> handle a lot of the low level target interface you're concerned with that
> we don't have at that level, but do at the TargetMachine level.
>
> Does this make sense?
>
>
To be fair TargetTuple might be the answer, but I don't see how it's
necessarily solving the issues with the MC layer interfaces rather than
simply adding a blob of data to them? Could you explain what the future
direction is for how this is going to work?

-eric


> Until LTO the serialization issue isn't much of an issue as anything that
> wants to deal with code generation in this way can create a TargetMachine
> via some of the existing code generation interfaces, it's most assuredly
> not ideal, but mostly hampered by the existing libLTO mechanism for passing
> options to the code generator. A serialization here might make sense but I
> haven't figured out if one is necessarily needed yet.
>
> -eric
>
>
> On Sat, Aug 8, 2015 at 6:58 AM Daniel Sanders <Daniel.Sanders at imgtec.com>
> wrote:
>
>> > > OK. What's the general class design look like then? The text from the
>> > > original mail was fairly confusing then as I thought he was doing
>> something
>> > > that you say he isn't doing :)
>> > Daniel, can you send your current plan for the Tuple class?
>>
>> Sorry for being a bit slow to answer this bit. It's been a busy week due
>> to my sister's wedding.
>>
>> The plan for TargetTuple's implementation is fairly brief since it is
>> heavily based on the current llvm::Triple but it has two main parts to it.
>> The first is fairly uninteresting and is to create a TargetTuple class with
>> (almost) the same interface as Triple and have this be the primary object
>> throughout the core libraries. This separates the GNU Triple from the
>> target description and gives us room to make changes to the target
>> description without changing the meaning of 'GNU Triple'. This is what the
>> patch I posted is starting and we've already covered this part of the plan
>> so I'll focus on the second part.
>>
>> The second part is to refactor the representation and tweak the
>> interface/implementation of TargetTuple into something more appropriate and
>> complete for a target description. As you'll see, these
>> implementation/interface changes compared to llvm::Triple are fairly minor.
>> The key improvement in the addition of the TargetTuple is in the ability to
>> represent the real target after tool options are taken into account and
>> llvm::Triple's methods are generally good as they are.
>>
>> The member variables will be similar in principle to llvm::Triple except
>> that the string form (if we have any at all following the discussion
>> elsewhere in this thread) will not be stored. This is because, unlike
>> Triple, we can reconstruct it from the fields. Member variables will be
>> split where they currently specify multiple things. For example, the
>> architecture field sometimes specifies endianness in values like
>> 'mips'/'mipsel' and these will be split into an architecture value 'mips'
>> and a separate endianness boolean. I don't think we need to handle
>> mixed-endian but we can use enums if necessary. I'm expecting to have the
>> following members in the base class by the end (all undefined types here
>> are enums and most are in the posted patch):
>>
>>   // The architecture with no additional details such as endianness.
>>   // Note that this is not the same ArchType as in llvm::Triple. For
>> example, 'arm' and 'armeb' are both
>>   // simply 'arm' in TargetTuple. Similarly for other endian variants and
>> things like 'mips'/'mips64' where
>>   // 'mips64' isn't a significantly different architecture from 'mips'
>>   ArchType Arch;
>>
>>   // The Architecture revision or variant
>>   // Like Arch, this isn't the same as the one in llvm::Triple since more
>> architectures ought to be using it than is necessary for llvm::Triple.
>>   // For example, Mips architecture revisions should be represented here
>> even though GNU triples don't state it. For completeness,
>>   // I should mention that there are some unconventional triples that do
>> mention Mips architecture revisions but we don't implement them at the
>> moment.
>>   SubArchType SubArch;
>>
>>   // The vendor.
>>   // This doesn't have to be the same as the one in llvm::Triple but I
>> see little reason to differ.
>>   VendorType Vendor;
>>
>>   // The OS.
>>   // This doesn't have to be the same as the one in llvm::Triple but I
>> see little reason to differ.
>>   OSType OS;
>>
>>   // The environment.
>>   // This doesn't have to be the same as the one in llvm::Triple but I
>> see little reason to differ.
>>   EnvironmentType Environment;
>>   unsigned MajorVersion, MinorVersion, MicroVersion;
>>
>>   // The object format (ELF, MachO, COFF, etc.) in use.
>>   ObjectFormatType ObjectFormat;
>>
>>   // The endianness. If we ever need mixed-endian or other variants we
>> can change this to an enum.
>>   bool BigEndian;
>>
>>   // The ABI. The value 'Standard' can be used for targets with only one
>> ABI.
>>   ABIType ABI;
>>
>>   // The SubABI. As far as I know, few architectures need this so it may
>> make more sense in a target specific subclass
>>   SubABIType SubABI;
>>
>> To be clear, the above list is based on the assumption that we can
>> serialize a TargetTuple in IR without including a GNU Triple. If we do want
>> to always write out the GNU Triple when writing IR then we will need to add
>> it to this list of members since we can't derive it from the rest of the
>> TargetTuple (we can have zero or multiple valid GNU Triples for a given
>> TargetTuple).
>>
>> As these members are introduced, we will re-implement the predicates that
>> we've carried over from llvm::Triple in terms of these members. The current
>> list of interface changes compared to llvm::Triple is below but note that
>> this list is subject to change:
>> * Add getters/setters for new members as necessary (e.g. for endian or
>> ABI).
>> * Remove normalize() since the representation of a TargetTuple is already
>> normalized.
>> * Assuming we don't want to write GNU Triples to IR, remove getTriple()
>> and setTriple() since we won't need them. Otherwise, keep them but make
>> them use an llvm::Triple instead of a string.
>> * Assuming we don't want a string form, remove str() as well.
>> * Rename isArch*bit() to arePointers*bit() which is all they really test.
>> The current name is misleading on some architectures since it implies that
>> architectures are inherently 16/32/64-bit but it's really a software
>> decision.
>> * Remove the get(Big|Little)EndianArchVariant() factory methods in favour
>> of a setEndian(...) (or similar) mutator.
>> * Likewise, remove get*bitArchVariant() and have the TargetParser handle
>> -m32/-m64 instead since it needs to call setArch() or setABI() depending on
>> the architecture.
>> * Remove getArchTypePrefix() since there shouldn't be anything that truly
>> needs the original string if our representation is good enough.
>> There are a few other minor tweaks that could probably be made but I've
>> left them out of this list because they are very target specific (e.g. OS X
>> version numbers need a special comparison predicate at the moment).
>>
>> For completeness, I should also mention that as a result of the
>> introduction of TargetTuple a number of member functions of Triple will
>> become unused. These will be deprecated and deleted.
>> Finally, at some point we'll need to introduce target specific
>> TargetTuple subclasses to add target specific data like ARM's FPU
>> information or MIPS's NaN encoding. At this point we'll have to add a
>> factory method to select the correct subclass based on the Arch member.
>> This should be a fairly easy change when we get to that point as there will
>> be very few invocations of the TargetTuple constructor left.
>>
>> Hopefully, that gives you a better picture of the end point for
>> TargetTuple. Overall, it's just a mutable version of Triple with some
>> additional target information and without the ambiguity found in GNU
>> Triples. Let me know if you still have questions.
>> ________________________________________
>> From: Renato Golin [renato.golin at linaro.org]
>> Sent: 30 July 2015 00:11
>> To: Eric Christopher
>> Cc: Daniel Sanders; LLVM Developers Mailing List (llvmdev at cs.uiuc.edu);
>> Jim Grosbach (grosbach at apple.com)
>> Subject: Re: The Trouble with Triples
>>
>> On 29 July 2015 at 23:47, Eric Christopher <echristo at gmail.com> wrote:
>> > This part doesn't seem obvious from the direction the patches are going.
>>
>> Until now, most of what he has done was to refactor the Triple class,
>> with no functional change, and to create a thin layer around the
>> Triple (called Tuple) and pass those instead. This is on par with that
>> premise.
>>
>> The current patch is the first one to actually have some more
>> substantial change, so it's good that you stopped it now, before we
>> start breaking everything.
>>
>> Maybe, knowing what it is now, if you could have another quick look at
>> the patch, and see if the new light has helped understand the patch
>> for what it will be. Maybe it's still not good enough, so then we'll
>> have to resort to a new round of design discussions.
>>
>>
>> > Definitely don't want this in the middle end at all. That all can be
>> part of
>> > the TargetMachine/TargetSubtargetInfo interface.
>>
>> Ah, yes! But the TargetMachine (& pals) are created from information
>> from the Triple and the other bits that front-ends keep for
>> themselves.
>>
>> So, in theory, if the Tuple is universal, creating them with a Tuple
>> (instead of a Triple+stuff) will free the front-ends of keeping the
>> rest of the info on their own, and TargetMachine/SubTargetInfo/etc
>> will be more homogeneous across different tools / front-ends than it
>> is today.
>>
>> Another strong point is: we're not trying to change any other machine
>> description / API. This is just about the user options and defaults,
>> that are used to *create* machine descriptions.
>>
>>
>> >> The decision to create a new class (Tuple) is because Triple already
>> >> has a legacy-heavy meaning, which should not change.
>> >
>> > Agreed with at least the "because" part.
>>
>> There was also the name. Triple is very far from the truth. :)
>>
>> But changing the Triple class could cause ripples in the mud that
>> would be hard to see at first, and hard to change later, after people
>> started relying on it.
>>
>> The final goal is that the Triple class would end up as being nothing
>> more than a Triple *parser*, with the current legacy logic, setting up
>> the Tuple fields and using them to select the rest of the default
>> fields.
>>
>>
>> > OK. What's the general class design look like then? The text from the
>> > original mail was fairly confusing then as I thought he was doing
>> something
>> > that you say he isn't doing :)
>>
>> Daniel, can you send your current plan for the Tuple class?
>>
>> cheers,
>> --renato
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150915/d55ec30f/attachment-0001.html>