[PATCH] Beginning of alloca implementation for Mips fast-isel

[Eric Christopher]

Some inline comments.


================
Comment at: lib/Target/Mips/MipsFastISel.cpp:1
@@ -1,2 +1,2 @@
 //===-- MipsastISel.cpp - Mips FastISel implementation
 //---------------------===//
----------------
Could you fix this comment (in a separate commit) while you're at it?

================
Comment at: lib/Target/Mips/MipsFastISel.cpp:9
@@ -7,2 +8,3 @@
 #include "MipsRegisterInfo.h"
+#include "MipsSEInstrInfo.h"
 #include "MipsSubtarget.h"
----------------
You don't need both includes.

================
Comment at: lib/Target/Mips/MipsFastISel.cpp:322
@@ -305,9 +321,3 @@
 
-bool MipsFastISel::computeAddress(const Value *Obj, Address &Addr) {
-  // This construct looks a big awkward but it is how other ports handle this
-  // and as this function is more fully completed, these cases which
-  // return false will have additional code in them.
-  //
-  if (isa<Instruction>(Obj))
-    return false;
-  else if (isa<ConstantExpr>(Obj))
+bool MipsFastISel::computeAddress(const Value *Obj, Address &Addr, Type *Ty) {
+
----------------
I might be missing something but Ty looks unused except for passing it down to other invocations of computeAddress?

================
Comment at: lib/Target/Mips/MipsFastISel.cpp:1358
@@ -1245,1 +1357,3 @@
 
+MipsFastISel::Address MipsFastISel::ensureOffsetFits(Address Addr,
+                                                     bool FitsOffset) {
----------------
This routine is just weirdly named/parametered. I'd probably just remove the boolean parameter and do the check inside. It'll make much more sense from the outside too.

================
Comment at: test/CodeGen/Mips/Fast-ISel/overflt.ll:10
@@ +9,3 @@
+ at .str = private unnamed_addr constant [5 x i8] c"%f \0A\00", align 1
+
+; Function Attrs: nounwind
----------------
Is this straight from C code or did you construct this up?

http://reviews.llvm.org/D6426

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