[PATCH] Implement shift ops for Mips fast-isel

[Daniel Sanders]

Vasileios is going to be taking over our Fast ISel work since Reed is needed elsewhere. Vasileios's (and my) first priority is to clear out the patch backlog, then he'll move on to cleaning things up and removing some unnecessary restrictions (e.g. PIC is currently required even for operations that don't involve addresses).

@vkalintiris: Thanks for sorting out the inconsistency between sign/zero-extension handling. That was the main issue I had with the patch.

Could you change the subject and description to be more commit-message like? Also, could you add something like 'Based on a patch by Reed Kotler' or 'Patch by Reed Kotler with some changes by Vasileios Kalintiris'. Reed should still get credit for his work.


================
Comment at: lib/Target/Mips/MipsFastISel.cpp:1390
@@ -1388,1 +1389,3 @@
 
+bool MipsFastISel::selectShift(const Instruction *I) {
+  MVT RetVT;
----------------
Nit: Could you add some appropriate vertical whitespace here? It's quite a long block of code at the moment.

================
Comment at: lib/Target/Mips/MipsFastISel.cpp:1402-1403
@@ +1401,4 @@
+    return false;
+  // if AShr then we need to make sure the operand0 is sign extended
+  //
+  if (Opcode == Instruction::AShr || Opcode == Instruction::LShr) {
----------------
Nit: Capitalization and punctuation. Also the second comment line is blank

================
Comment at: test/CodeGen/Mips/Fast-ISel/shftopm.ll:20
@@ +19,3 @@
+  store i16 %shl, i16* @s3, align 2
+; CHECK:        .ent    sll
+; CHECK:        lui     $[[REG_GPa:[0-9]+]], %hi(_gp_disp)
----------------
Nit: Could you switch this over to CHECK-LABEL? Also, it's more usual to check for 'sll:'.

================
Comment at: test/CodeGen/Mips/Fast-ISel/shftopm.ll:29-30
@@ +28,4 @@
+; CHECK-DAG:    lhu     $[[S2:[0-9]+]], 0($[[S2_ADDR]])
+; CHECK:	sllv	$[[RES:[0-9]+]], $[[S1]], $[[S2]]
+; CHECK:	sh	$[[RES]], 0($[[S3_ADDR]])
+; CHECK:        .end    sll
----------------
Nit: Indentation. Likewise for a few other cases below

================
Comment at: test/CodeGen/Mips/Fast-ISel/shftopm.ll:31
@@ +30,3 @@
+; CHECK:	sh	$[[RES]], 0($[[S3_ADDR]])
+; CHECK:        .end    sll
+  ret void
----------------
Nit: Not necessary if we use CHECK-LABEL for the start label.

================
Comment at: test/CodeGen/Mips/Fast-ISel/shftopm.ll:136-208
@@ +135,75 @@
+
+; Function Attrs: nounwind
+;define i32 @main() #0 {
+;entry:
+;  %retval = alloca i32, align 4
+;  store i32 0, i32* %retval
+;  call void @sll()
+;  %0 = load i16* @s3, align 2
+;  %conv = sext i16 %0 to i32
+;  %cmp = icmp ne i32 %conv, -1424
+;  br i1 %cmp, label %if.then, label %if.end
+;
+;if.then:                                          ; preds = %entry
+;  call void @abort() #2
+;  unreachable
+;
+;if.end:                                           ; preds = %entry
+;  call void @slli()
+;  %1 = load i16* @s3, align 2
+;  %conv2 = sext i16 %1 to i32
+;  %cmp3 = icmp ne i32 %conv2, -2848
+;  br i1 %cmp3, label %if.then5, label %if.end6
+;
+;if.then5:                                         ; preds = %if.end
+;  call void @abort() #2
+;  unreachable
+;
+;if.end6:                                          ; preds = %if.end
+;  call void @srl()
+;  %2 = load i16* @us3, align 2
+;  %conv7 = zext i16 %2 to i32
+;  %cmp8 = icmp ne i32 %conv7, 2032
+;  br i1 %cmp8, label %if.then10, label %if.end11
+;
+;if.then10:                                        ; preds = %if.end6
+;  call void @abort() #2
+;  unreachable
+;
+;if.end11:                                         ; preds = %if.end6
+;  call void @srli()
+;  %3 = load i16* @us3, align 2
+;  %conv12 = zext i16 %3 to i32
+;  %cmp13 = icmp ne i32 %conv12, 4064
+;  br i1 %cmp13, label %if.then15, label %if.end16
+;
+;if.then15:                                        ; preds = %if.end11
+;  call void @abort() #2
+;  unreachable
+;
+;if.end16:                                         ; preds = %if.end11
+;  call void @sra()
+;  %4 = load i16* @s3, align 2
+;  %conv17 = sext i16 %4 to i32
+;  %cmp18 = icmp ne i32 %conv17, -6
+;  br i1 %cmp18, label %if.then20, label %if.end21
+;
+;if.then20:                                        ; preds = %if.end16
+;  call void @abort() #2
+;  unreachable
+;
+;if.end21:                                         ; preds = %if.end16
+;  call void @srai()
+;  %5 = load i16* @s3, align 2
+;  %conv22 = sext i16 %5 to i32
+;  %cmp23 = icmp ne i32 %conv22, -23
+;  br i1 %cmp23, label %if.then25, label %if.end26
+;
+;if.then25:                                        ; preds = %if.end21
+;  call void @abort() #2
+;  unreachable
+;
+;if.end26:                                         ; preds = %if.end21
+;  ret i32 0
+;}
+
----------------
Nit: Please remove this. It's not adding anything to the test.

================
Comment at: test/CodeGen/Mips/Fast-ISel/shftopm.ll:209-217
@@ +208,10 @@
+;}
+
+; Function Attrs: noreturn
+declare void @abort() #1
+
+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" }
+attributes #1 = { noreturn "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" }
+attributes #2 = { noreturn }
+
+
----------------
Nit: We should be able to remove this too.

http://reviews.llvm.org/D6726

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