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

reed kotler rkotler at mips.com
Tue Feb 17 18:41:59 PST 2015


Will upload changed source reflecting the latest comments I posted.


================
Comment at: lib/Target/Mips/MipsFastISel.cpp:664
@@ -608,2 +663,3 @@
   if (Addr.isRegBase()) {
+    ensureOffset(Addr, isInt<16>(Addr.getOffset()));
     emitInstStore(Opc, SrcReg, Addr.getReg(), Addr.getOffset());
----------------
dsanders wrote:
> It's not very obvious that we're changing Addr here (and at the other points this is called). Returning the new reference would be clearer
I've made the change so that it returns an Address but personally I think that this makes it harder to read and I don't see why it's confusing to modify a reference variable in C++, that's what they are there for.

================
Comment at: lib/Target/Mips/MipsFastISel.cpp:1371
@@ -1314,1 +1370,3 @@
 
+void MipsFastISel::ensureOffset(Address &Addr, bool FitsOffset) {
+  if (FitsOffset)
----------------
dsanders wrote:
> Nit: What are we ensuring about the offset? Shouldn't we use ensureOffsetWithinRange or something along those lines?
I've changed the name to ensureOffsetFits.

If the offset does not fit, then the integer is materialized and the signness is taken care of there.

http://reviews.llvm.org/D6426

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






More information about the llvm-commits mailing list