[PATCH] [PPC64] Properly handle the mftb instruction

[Nemanja Ivanovic]

================
Comment at: lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp:1189
@@ +1188,3 @@
+    if (STI.getFeatureBits()[PPC::DeprecatedMFTB]) {
+      assert(Inst.getNumOperands() == 2 && "Expecting two operands");
+      
----------------
I am probably missing something since the test case you have does test the mnemonic with a single operand, but I'm curious how come we don't throw this assert when processing lines 50 and 61 in the test case. Does the immediate operand get added to the DAG before this?

================
Comment at: lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp:1195
@@ +1194,3 @@
+      TmpInst.addOperand(Inst.getOperand(1));
+      Inst = TmpInst;
+    }
----------------
I'm just curious about why the temp is needed. From the standpoint of readability, it would seem to make sense to call 
```
Inst.setOpcode(PPC::MFSPR);
```
then the remaining modification and copy-assignment is not necessary. Unless of course there is a reason the temporary is needed (i.e. Inst is of a type that does not have setOpcode, etc.).

================
Comment at: lib/Target/PowerPC/PPC.td:250
@@ -249,3 +249,3 @@
 
-def : Processor<"generic", G3Itineraries, [Directive32]>;
+def : Processor<"generic", G3Itineraries, [Directive32, DeprecatedMFTB]>;
 def : ProcessorModel<"440", PPC440Model, [Directive440, FeatureISEL,
----------------
This is really nit-picking, but leaving the name the same seems misleading. We are essentially "un-deprecating" the instruction and adding the DeprecatedMFTB to almost every processor.
I do realize that the instruction itself will not be emitted, but a different mnemonic, but this may not be clear to someone looking at this 5 years from now.

http://reviews.llvm.org/D10419

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/