[PATCH] Add bulk of returning of values to Mips fast-isel

Daniel Sanders daniel.sanders at imgtec.com
Thu Feb 12 08:24:46 PST 2015


This patch LGTM. I do still have some concerns about the way our SelectionDAG and FastISel are handling sign/zero extended types in the calling convention differently but that shouldn't block this patch.

Also, please follow up on the overloaded function issue as soon as you can.


================
Comment at: lib/Target/Mips/MipsFastISel.cpp:994-996
@@ +993,5 @@
+
+  if (F.isVarArg())
+      return false;
+
+  // Build a list of return value registers.
----------------
dsanders wrote:
> Why is this needed? Variable argument lists don't have any effect on the handling of return values.
Done

================
Comment at: lib/Target/Mips/MipsFastISel.cpp:1006
@@ +1005,3 @@
+    SmallVector<CCValAssign, 16> ValLocs;
+    CCState CCInfo(CC, F.isVarArg(), *FuncInfo.MF, ValLocs, I->getContext());
+    CCAssignFn *RetCC = RetCC_Mips;
----------------
dsanders wrote:
> This ought to be a MipsCCState object
Done

================
Comment at: lib/Target/Mips/MipsFastISel.cpp:1197
@@ -1119,1 +1196,3 @@
+  bool Success = emitIntExt(SrcVT, SrcReg, DestVT, DestReg, isZExt);
+  return Success? DestReg: 0;
 }
----------------
dsanders wrote:
> Nit: Space before '?'
Done

================
Comment at: test/CodeGen/Mips/Fast-ISel/retabi.ll:13
@@ +12,3 @@
+entry:
+; CHECK-LABEL: reti
+  %0 = load i32* @i, align 4
----------------
dsanders wrote:
> Nit: For consistency with other tests we should probably include the colon following the label name.
Done

================
Comment at: test/CodeGen/Mips/Fast-ISel/retabi.ll:16
@@ +15,3 @@
+  ret i32 %0
+; CHECK:    	lui	$[[REG_GPa:[0-9]+]], %hi(_gp_disp)
+; CHECK:	addiu	$[[REG_GPb:[0-9]+]], $[[REG_GPa]], %lo(_gp_disp)
----------------
dsanders wrote:
> Nit: indentation
> 
> Likewise for all the lui lines below
Done

================
Comment at: test/CodeGen/Mips/Fast-ISel/retabi.ll:49-50
@@ +48,4 @@
+; CHECK:	lw	$[[REG_C_ADDR:[0-9]+]], %got(c)($[[REG_GP]])
+; CHECK:	lbu	$[[REG_C:[0-9]+]], 0($[[REG_C_ADDR]])
+; CHECK:	seb	$2, $[[REG_C]]
+; CHECK:	jr	$ra
----------------
dsanders wrote:
> I think I already know the answer to this question but why not use lb instead of lbu+seb?
Already answered above.

================
Comment at: test/CodeGen/Mips/Fast-ISel/retabi.ll:81-82
@@ +80,3 @@
+}
+
+
----------------
dsanders wrote:
> Nit: blank lines at EOF
Done

http://reviews.llvm.org/D5920

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






More information about the llvm-commits mailing list