<div dir="ltr">resending to the right address for the mailing list.<br><br><div class="gmail_quote"><div dir="ltr">---------- Forwarded message ---------<br>From: Eric Christopher <<a href="mailto:[email protected]">[email protected]</a>><br>Date: Tue, Sep 15, 2015 at 9:11 AM<br>Subject: Re: The Trouble with Triples<br>To: Daniel Sanders <<a href="mailto:[email protected]">[email protected]</a>>, Renato Golin <<a href="mailto:[email protected]">[email protected]</a>><br>Cc: LLVM Developers Mailing List (<a href="mailto:[email protected]">[email protected]</a>) <<a href="mailto:[email protected]">[email protected]</a>>, Jim Grosbach (<a href="mailto:[email protected]">[email protected]</a>) <<a href="mailto:[email protected]">[email protected]</a>><br></div><br><br><div dir="ltr"><div>I was writing a rather longer email with an actual plan for you, but as you've committed I'll send it briefly.</div><div><br></div>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.<div><br></div><div>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:</div><div><br></div><div>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<br><div><br></div><div>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.</div><div><br></div><div>Does this make sense?</div><div><br></div><div>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.</div></div></div><div dir="ltr"><div><div><br></div><div>-eric</div></div></div><div dir="ltr"><div><div><br></div><div><br></div><div><div class="gmail_quote"><div dir="ltr">On Sat, Aug 8, 2015 at 6:58 AM Daniel Sanders <<a href="mailto:[email protected]" target="_blank">[email protected]</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">> > OK. What's the general class design look like then? The text from the<br>
> > original mail was fairly confusing then as I thought he was doing something<br>
> > that you say he isn't doing :)<br>
> Daniel, can you send your current plan for the Tuple class?<br>
<br>
Sorry for being a bit slow to answer this bit. It's been a busy week due to my sister's wedding.<br>
<br>
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.<br>
<br>
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.<br>
<br>
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):<br>
<br>
  // The architecture with no additional details such as endianness.<br>
  // Note that this is not the same ArchType as in llvm::Triple. For example, 'arm' and 'armeb' are both<br>
  // simply 'arm' in TargetTuple. Similarly for other endian variants and things like 'mips'/'mips64' where<br>
  // 'mips64' isn't a significantly different architecture from 'mips'<br>
  ArchType Arch;<br>
<br>
  // The Architecture revision or variant<br>
  // 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.<br>
  // For example, Mips architecture revisions should be represented here even though GNU triples don't state it. For completeness,<br>
  // I should mention that there are some unconventional triples that do mention Mips architecture revisions but we don't implement them at the moment.<br>
  SubArchType SubArch;<br>
<br>
  // The vendor.<br>
  // This doesn't have to be the same as the one in llvm::Triple but I see little reason to differ.<br>
  VendorType Vendor;<br>
<br>
  // The OS.<br>
  // This doesn't have to be the same as the one in llvm::Triple but I see little reason to differ.<br>
  OSType OS;<br>
<br>
  // The environment.<br>
  // This doesn't have to be the same as the one in llvm::Triple but I see little reason to differ.<br>
  EnvironmentType Environment;<br>
  unsigned MajorVersion, MinorVersion, MicroVersion;<br>
<br>
  // The object format (ELF, MachO, COFF, etc.) in use.<br>
  ObjectFormatType ObjectFormat;<br>
<br>
  // The endianness. If we ever need mixed-endian or other variants we can change this to an enum.<br>
  bool BigEndian;<br>
<br>
  // The ABI. The value 'Standard' can be used for targets with only one ABI.<br>
  ABIType ABI;<br>
<br>
  // The SubABI. As far as I know, few architectures need this so it may make more sense in a target specific subclass<br>
  SubABIType SubABI;<br>
<br>
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).<br>
<br>
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:<br>
* Add getters/setters for new members as necessary (e.g. for endian or ABI).<br>
* Remove normalize() since the representation of a TargetTuple is already normalized.<br>
* 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.<br>
* Assuming we don't want a string form, remove str() as well.<br>
* 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.<br>
* Remove the get(Big|Little)EndianArchVariant() factory methods in favour of a setEndian(...) (or similar) mutator.<br>
* Likewise, remove get*bitArchVariant() and have the TargetParser handle -m32/-m64 instead since it needs to call setArch() or setABI() depending on the architecture.<br>
* Remove getArchTypePrefix() since there shouldn't be anything that truly needs the original string if our representation is good enough.<br>
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).<br>
<br>
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.<br>
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.<br>
<br>
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.<br>
________________________________________<br>
From: Renato Golin [<a href="mailto:[email protected]" target="_blank">[email protected]</a>]<br>
Sent: 30 July 2015 00:11<br>
To: Eric Christopher<br>
Cc: Daniel Sanders; LLVM Developers Mailing List (<a href="mailto:[email protected]" target="_blank">[email protected]</a>); Jim Grosbach (<a href="mailto:[email protected]" target="_blank">[email protected]</a>)<br>
Subject: Re: The Trouble with Triples<br>
<br>
On 29 July 2015 at 23:47, Eric Christopher <<a href="mailto:[email protected]" target="_blank">[email protected]</a>> wrote:<br>
> This part doesn't seem obvious from the direction the patches are going.<br>
<br>
Until now, most of what he has done was to refactor the Triple class,<br>
with no functional change, and to create a thin layer around the<br>
Triple (called Tuple) and pass those instead. This is on par with that<br>
premise.<br>
<br>
The current patch is the first one to actually have some more<br>
substantial change, so it's good that you stopped it now, before we<br>
start breaking everything.<br>
<br>
Maybe, knowing what it is now, if you could have another quick look at<br>
the patch, and see if the new light has helped understand the patch<br>
for what it will be. Maybe it's still not good enough, so then we'll<br>
have to resort to a new round of design discussions.<br>
<br>
<br>
> Definitely don't want this in the middle end at all. That all can be part of<br>
> the TargetMachine/TargetSubtargetInfo interface.<br>
<br>
Ah, yes! But the TargetMachine (& pals) are created from information<br>
from the Triple and the other bits that front-ends keep for<br>
themselves.<br>
<br>
So, in theory, if the Tuple is universal, creating them with a Tuple<br>
(instead of a Triple+stuff) will free the front-ends of keeping the<br>
rest of the info on their own, and TargetMachine/SubTargetInfo/etc<br>
will be more homogeneous across different tools / front-ends than it<br>
is today.<br>
<br>
Another strong point is: we're not trying to change any other machine<br>
description / API. This is just about the user options and defaults,<br>
that are used to *create* machine descriptions.<br>
<br>
<br>
>> The decision to create a new class (Tuple) is because Triple already<br>
>> has a legacy-heavy meaning, which should not change.<br>
><br>
> Agreed with at least the "because" part.<br>
<br>
There was also the name. Triple is very far from the truth. :)<br>
<br>
But changing the Triple class could cause ripples in the mud that<br>
would be hard to see at first, and hard to change later, after people<br>
started relying on it.<br>
<br>
The final goal is that the Triple class would end up as being nothing<br>
more than a Triple *parser*, with the current legacy logic, setting up<br>
the Tuple fields and using them to select the rest of the default<br>
fields.<br>
<br>
<br>
> OK. What's the general class design look like then? The text from the<br>
> original mail was fairly confusing then as I thought he was doing something<br>
> that you say he isn't doing :)<br>
<br>
Daniel, can you send your current plan for the Tuple class?<br>
<br>
cheers,<br>
--renato<br>
</blockquote></div></div></div></div></div></div>