[PATCH] Implement FastMaterializeAlloca in Mips fast-isel

[Daniel Sanders]

LGTM with some changes


================
Comment at: lib/Target/Mips/MipsFastISel.cpp:260-263
@@ +259,6 @@
+
+  // Don't handle dynamic allocas.
+  if (!FuncInfo.StaticAllocaMap.count(AI))
+    return 0;
+
+  DenseMap<const AllocaInst *, int>::iterator SI =
----------------
This is redundant. DenseMapBase<...>::find() returns DenseMapBase<...>::end() when the key isn't found so there's no need to separately count(AI) and find(AI)

================
Comment at: test/CodeGen/Mips/Fast-ISel/fastalloca.ll:4-7
@@ +3,6 @@
+
+; ModuleID = 'fooa.c'
+target datalayout = "e-m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64"
+target triple = "mipsel--linux-gnu"
+
+%struct.x = type { i32 }
----------------
Please drop these four lines from the test. It's better to specify the triple on the command line and using the default datalayout will be better if it ever needs to change.

(sorry for the duplicate comment, Phabricator won't let me delete either of them for some reason)

================
Comment at: test/CodeGen/Mips/Fast-ISel/fastalloca.ll:16-29
@@ +15,16 @@
+; CHECK-LABEL: foobar:
+  %retval = alloca i32, align 4
+  %x.addr = alloca i32, align 4
+  %a = alloca %struct.x, align 4
+  %c = alloca %struct.x*, align 4
+  store i32 %x, i32* %x.addr, align 4
+  %x1 = getelementptr inbounds %struct.x, %struct.x* %a, i32 0, i32 0
+  %0 = load i32, i32* %x.addr, align 4
+  store i32 %0, i32* %x1, align 4
+  store %struct.x* %a, %struct.x** %c, align 4
+  %1 = load %struct.x*, %struct.x** %c, align 4
+  %x2 = getelementptr inbounds %struct.x, %struct.x* %1, i32 0, i32 0
+  %2 = load i32, i32* %x2, align 4
+  store i32 %2, i32* @i, align 4
+  %3 = load i32, i32* %retval
+; CHECK-DAG: 	lw	$[[I_ADDR:[0-9]+]], %got(i)($[[REG_GP:[0-9]+]])
----------------
I think we could simplify this test quite a lot. We only want to test alloca instructions so I think we could do something like:
  define i32 @foo(i32* %d) {
    %0 = alloca i32, align 4
    store i32 %0, i32* %d, align 4
  }
and just check that appropriate addiu's and sw's are created. Obviously, we'd want to test a few layouts.

That said, we've discussed (off-list) your plan to migrate these tests to a style similar to the test/CodeGen/Mips/llvm-ir tests (which tries to test each LLVM-IR instruction with as few dependencies as possible) once the backlog is cleared and I'm happy for this kind of change to be made at that point.

================
Comment at: test/CodeGen/Mips/Fast-ISel/fastalloca.ll:31
@@ +30,3 @@
+; CHECK-DAG: 	lw	$[[I_ADDR:[0-9]+]], %got(i)($[[REG_GP:[0-9]+]])
+; CHECK-DAG:	addiu	$[[A_ADDR:[0-9]+]], $fp, 8
+; CHECK-DAG:    sw      $[[A_ADDR]], [[A_ADDR_FI:[0-9]+]]($fp)
----------------
Nit: indentation

================
Comment at: test/CodeGen/Mips/Fast-ISel/fastalloca.ll:39-57
@@ +38,20 @@
+
+; Function Attrs: nounwind
+define i32 @main() #0 {
+entry:
+; CHECK-LABEL: main:
+  %retval = alloca i32, align 4
+  store i32 0, i32* %retval
+  %call = call i32 @foobar(i32 signext 99)
+  %0 = load i32, i32* @i, align 4
+  %sub = sub nsw i32 99, %0
+  ret i32 %sub
+}
+
+attributes #0 = { nounwind "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.module.flags = !{!0}
+!llvm.ident = !{!1}
+
+!0 = !{i32 1, !"PIC Level", i32 2}
+!1 = !{!"clang version 3.6.0 (gitosis at dmz-portal.mips.com:clang.git f96fd4c68e89695cc64686bcf0dc768aee4638e5) (gitosis at dmz-portal.mips.com:llvm.git 33df92e7a51bb034540e4de79d2d4cff83439d7b)"}
----------------
Nit: Not needed.

================
Comment at: test/CodeGen/Mips/Fast-ISel/fastalloca.ll:3-6
@@ +2,6 @@
+; RUN:     < %s | FileCheck %s
+
+; ModuleID = 'fooa.c'
+target datalayout = "e-m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64"
+target triple = "mipsel--linux-gnu"
+
----------------
Please drop these four lines from the test. It's better to specify the triple on the command line and using the default datalayout will be better if it ever needs to change.

http://reviews.llvm.org/D6742

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