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

[Renato Golin]

Hi Daniel,

I have only looked at the header and implementation of TargetTuple, and I have a few comments on comments. :)

I'll continue looking at the other changes in due time.

cheers,
--renato


================
Comment at: include/llvm/ADT/TargetTuple.h:39
@@ +38,3 @@
+  //        would be fairly sensible to have a single 'mips' architecture and
+  //        distinguish endianness and ABI elsewhere.
+  enum ArchType {
----------------
I absolutely agree!

================
Comment at: include/llvm/ADT/TargetTuple.h:65
@@ +64,3 @@
+    tce,        // TCE (http://tce.cs.tut.fi/): tce
+    thumb,      // Thumb (little endian): thumb, thumbv.*
+    thumbeb,    // Thumb (big endian): thumbeb
----------------
This has to go soon. Please add a FIXME to make sure we merge this into arm/armeb.

================
Comment at: include/llvm/ADT/TargetTuple.h:87
@@ +86,3 @@
+
+  // FIXME: Don't duplicate Triple::SubArchType.
+  enum SubArchType {
----------------
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.

================
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) {}
----------------
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.


================
Comment at: include/llvm/ADT/TargetTuple.h:284
@@ +283,3 @@
+
+  const std::string &getTriple() const { return GnuTT.str(); }
+
----------------
why two?

================
Comment at: lib/Support/TargetTuple.cpp:16
@@ +15,3 @@
+
+static TargetTuple::ArchType
+convertTripleArchToTupleArch(Triple::ArchType Arch) {
----------------
  // FIXME: These should go as soon as Triple is dead

Just to be sure people don't come here and say "OMG! WTF?".


http://reviews.llvm.org/D10969