[PATCH] [mips] Add assembler support for the .cprestore directive.

Daniel Sanders daniel.sanders at imgtec.com
Tue Jun 23 06:09:21 PDT 2015


LGTM with a couple nits and a more concept-focused spelling for the new predicate.


================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1761
@@ +1760,3 @@
+       Inst.getOpcode() == Mips::JalTwoReg || ExpandedJalSym) &&
+      isPicAndNotNxxAbi()) {
+    if (IsCpRestoreSet) {
----------------
This spelling says the same thing as the condition did. The intent was to name the predicate after the concept being tested rather than the mechanical values being tested.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1777-1779
@@ +1776,5 @@
+
+    } else {
+      Warning(IDLoc, "no .cprestore used in PIC mode");
+    }
+  }
----------------
Nit: Redundant braces.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2650-2653
@@ +2649,6 @@
+  MCInst MemInst;
+  if (IsLoad)
+    MemInst.setOpcode(Mips::LW);
+  else
+    MemInst.setOpcode(Mips::SW);
+
----------------
Nit: Use ternary operator

http://reviews.llvm.org/D6267

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






More information about the llvm-commits mailing list