[PATCH] D10969: Replace Triple with a new TargetTuple in MCTargetDesc/* and related. NFC.

[Daniel Sanders]

================
Comment at: include/llvm/ADT/TargetTuple.h:87
@@ +86,3 @@
+
+  // FIXME: Don't duplicate Triple::SubArchType.
+  enum SubArchType {
----------------
rengolin wrote:
> These enums will become authoritative once Triple is gone. 
> 
> Even if we don't remove Triple forever, it must use these, and not its own, anyway.
Ok, I'll drop the FIXME for these but keep the one for ArchType for now.

================
Comment at: include/llvm/ADT/TargetTuple.h:210
@@ +209,3 @@
+  /// ABI. A distributor may want the default to be N32 on low-memory targets
+  /// and would modify this constructor accordingly.
+  explicit TargetTuple(const Triple &GnuTT) : GnuTT(GnuTT) {}
----------------
rengolin wrote:
> I don't think we should encourage people re-writing the constructor, or it would be a nightmare to investigate issues.
> 
> One could use the default constructor and set ABI to 32-bits later, which would be a simpler patch to keep for both vendor and community.
> 
This is trying to account for configure-option ambiguity in the GNU triples. For GCC, it's fairly common for distributors to change the meanings of triples using options like --with-arch, --with-cpu, --with-abi, etc. There are some target-specific options such as --with-mode to select ARM/Thumb by default and --with-nan for MIPS NAN encoding selection. I'm not sure about other targets but the MIPS options are in very common use and is likely to get more common in the near future as we try to push towards IEEE754(2008) being the conventional floating point standard on our target (we're still on 1985 for the majority of software).

The thinking behind this comment is that that distributors need to do this kind of thing to make LLVM's GNU triples consistent with the rest of their distribution. We can therefore save ourselves a fair bit of pain by nominating the right place to do it. The advantage, is that LLVM (and related tools) can be made to have the same Triple meaning as the rest of the distribution. The downside is that, as you say, it makes investigating issues more difficult on distributions that do change it since they'll have a different default TargetTuple for the same Triple. TargetTuples are still unique and authoratative so we can compensate for this by using the tuple to investigate issues, effectively bypassing any customizations the distributor made in this constructor. Frontends may need to add command line options to allow TargetTuple's (or we could claim an unused subset of the triple strings) but that should be easy. We don't need to do the same thing for backends/LTO since we'll need the TargetTuple to be serialized in the LLVM-IR to avoid information loss anyway.

================
Comment at: include/llvm/ADT/TargetTuple.h:284
@@ +283,3 @@
+
+  const std::string &getTriple() const { return GnuTT.str(); }
+
----------------
rengolin wrote:
> why two?
At the moment, its just to preserve Triple's interface which had both. I've been rewriting instances of .getTriple() to .str() as I notice them. At some point, I'd like to rewrite the remainder and delete getTriple().

Triple::getTriple() is fairly common, but it's also the least clear of the two. The name sounds like it ought to return 'this' :-)

================
Comment at: lib/Support/TargetTuple.cpp:16
@@ +15,3 @@
+
+static TargetTuple::ArchType
+convertTripleArchToTupleArch(Triple::ArchType Arch) {
----------------
rengolin wrote:
>   // FIXME: These should go as soon as Triple is dead
> 
> Just to be sure people don't come here and say "OMG! WTF?".
Sure, I've paraphrased a bit in the update (which I'll post shortly) since it's really about replacing the Triple member (GnuTT).


http://reviews.llvm.org/D10969