[PATCH] [PATCH][mips] .abicalls and .set pic0 support

Jack Carter jack.carter at imgtec.com
Mon Jan 6 12:20:00 PST 2014



================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2490
@@ +2489,3 @@
+            "unexpected token in .option pic0 directive");
+      Parser.eatToEndOfStatement();
+    }
----------------
Rafael Ávila de Espíndola wrote:
> There is a "return true" missing in the error path, no?
Returning "true" tells the base parser that it needs to handle the error. We do not want that to happen when the target specific parser catches the problem and creates the error/warning pointing at the specific location of the problem.

I made this same mistake earlier and was corrected by my team.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2564
@@ +2563,3 @@
+      // Clear line
+      Parser.eatToEndOfStatement();
+    }
----------------
Rafael Ávila de Espíndola wrote:
> missing a return true in here, no?
Same as above

================
Comment at: lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp:27
@@ -25,3 +26,3 @@
 
-// pin vtable to this file
+// Pin vtable to this file.
 void MipsTargetStreamer::anchor() {}
----------------
Rafael Ávila de Espíndola wrote:
> It is OK to leave this in, but in the future please just commit independent cleanups like this independently. They qualify for post commit review.
Actually all my Mips/Target commits qualify for post commit review. We are posting this one for pre  commit as a courtesy to you.

That said,  I will attempt to address all incidental existing formatting and typo changes in their own patches.


http://llvm-reviews.chandlerc.com/D2446

BRANCH
  master

ARCANIST PROJECT
  llvm



More information about the llvm-commits mailing list