[PATCH] D19716: [mips] Use MipsMCExpr instead of MCSymbolRefExpr for all relocations.

[Daniel Sanders via llvm-commits]

dsanders added inline comments.

================
Comment at: lib/Target/Mips/InstPrinter/MipsInstPrinter.cpp:153
@@ -181,6 +152,3 @@
 
-  if ((Kind == MCSymbolRefExpr::VK_Mips_GPOFF_HI) ||
-      (Kind == MCSymbolRefExpr::VK_Mips_GPOFF_LO))
-    OS << ")))";
-  else if (Kind != MCSymbolRefExpr::VK_None)
+  if (Kind != MCSymbolRefExpr::VK_None)
     OS << ')';
----------------
sdardis wrote:
> Am I missing something here? As far as I can see, you assert that Kind == VK_None on line 143 but here you're testing if Kind != VK_None, which I believe is always false.
It looks like I missed this when I finished emptying out the switch statement. I'll make this unconditional.

================
Comment at: lib/Target/Mips/MCTargetDesc/MipsMCExpr.h:85-86
@@ +84,4 @@
+  bool isGpOff() const {
+    MipsExprKind Kind;
+    return isGpOff(Kind);
+  }
----------------
sdardis wrote:
> Should this be 'return isGpOff(Kind);' as the local declaration of 'Kind' here shadows the class definition.
This is intended to discard the MipsExprKind result from the other isGpOff predicate. I can call it 'Unused' if that's preferable.


http://reviews.llvm.org/D19716