[PATCH] D17373: [mips][microMIPS] Prevent usage of OR16_MMR6 instruction when code for microMIPS is generated.

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 29 08:08:06 PST 2016


dsanders added a comment.

In http://reviews.llvm.org/D17373#364232, @milena.vujosevic.janicic wrote:

> I put MicroMipsR6Inst16 on POOL16C_OR16_XOR16_FM_MMR6, as you suggested.
>
> The test-case you suggested would not work since it would always generate right OR16_MM instruction. However, I managed to simplify a bit the test case used last time.
>
> The test case you suggested does not trigger this bug due to the way FastISel functions. It always first calls method FastISel::selectOperator which can do fast instruction selection for a small set of operators. Then,  if selectOperator fails, FastISel calls MipsFastISel::fastSelectInstruction which checks if the architecture for which the code is generated is supported. If FastISel fails for one instruction, it does not continue trying for other instructions in that basic block. It selects instructions in reverse order.
>
> In the test-case you suggested, FastISel tries to select an instruction for RET by selectOperator method and it fails. Then it tries to select an instruction by fastSelectInstruction, which again fails since microMips is not supported architecture, and then FastISel stops and instruction selection is continued in a normal way (and there is no bug).
>
> In the test-case suggested in this patch, all instructions before OR are selected by selectOperator method, and then this method selects (wrong) OR16_MMR6 instruction.


In that case we just need to split the bb in the small test case. The code below emits an OR16_MMR6 on my build.


================
Comment at: test/CodeGen/Mips/micromips-or16.ll:3-43
@@ -2,14 +2,43 @@
 ; RUN:   -relocation-model=pic -O3 < %s | FileCheck %s
+; RUN: llc -O0 -march=mips -mcpu=mips32r2 -mattr=+micromips \
+; RUN:  -asm-show-inst < %s | FileCheck %s
+
+declare i32 @m(i32 signext %a)
+
+define i32 @f() {
+entry:
+  %retval = alloca i32, align 4
+  %a = alloca i32, align 4
+  store i32 0, i32* %retval, align 4
+  %0 = load i32, i32* %a, align 4
+  %cmp = icmp eq i32 %0, 0
+  br i1 %cmp, label %if.then, label %if.end
+
+if.then:                                          ; preds = %entry
+; CHECK-LABEL: f
+; CHECK-NOT: OR16_MMR6
+  %1 = load i32, i32* %a, align 4
+  %2 = load i32, i32* %a, align 4
+  %add = add nsw i32 %2, 1
+  %or = or i32 %1, %add
+  %call = call i32 @m(i32 signext %or)
+  br label %if.end
+
+if.end:                                           ; preds = %if.then, %entry
+  ret i32 0
+}
 
 define i32 @main() {
 entry:
+; CHECK-LABEL: main
+; CHECK: or16
   %retval = alloca i32, align 4
   %a = alloca i32, align 4
   %b = alloca i32, align 4
   %c = alloca i32, align 4
   store i32 0, i32* %retval
   %0 = load i32, i32* %b, align 4
   %1 = load i32, i32* %c, align 4
   %or = or i32 %0, %1
   store i32 %or, i32* %a, align 4
   ret i32 0
----------------
This is the simplest test I can find that emits an OR16_MMR6:
  define i32 @f(i32 signext %a, i32 signext %b) {
         %1 = or i32 %a, %b
         br label %b1
  b1:
         ret i32 %1
  }



http://reviews.llvm.org/D17373





More information about the llvm-commits mailing list