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

Renato Golin renato.golin at linaro.org
Tue Jul 7 07:13:05 PDT 2015


Phew! Ok, looks ok to me, with the comments.

About the Tuple+CPU+Features (and others), unless you're planning to do that move in this streak, I think it'd be best to keep a small FIXME comment to use CPU/Features/Options in the Tuple itself.


================
Comment at: include/llvm/Support/TargetRegistry.h:119
@@ -117,1 +118,3 @@
+                                              const TargetTuple &TT,
+                                              StringRef CPU);
   typedef MCTargetAsmParser *(*MCAsmParserCtorTy)(
----------------
I think this one deserves a FIXME. CPU should be already uniquely identifiable in the tuple.

================
Comment at: include/llvm/Support/TargetRegistry.h:351
@@ -348,3 +350,3 @@
       return nullptr;
-    return MCSubtargetInfoCtorFn(Triple(TheTriple), CPU, Features);
+    return MCSubtargetInfoCtorFn(TargetTuple(Triple(TT)), CPU, Features);
   }
----------------
Same here. I'd expect (in the future) something like this:

    TargetTuple Tuple (Triple(TT)); // Triple for now, TT directly later
    Tuple.setCPU(CPU);
    Tuple.setFeatures(Features);
    return MCSubtargetInfoCtorFn(Tuple);


http://reviews.llvm.org/D10969







More information about the llvm-commits mailing list