[MIPS] [microMIPS] Fix for filling delay slots for PseudoIndirectBranch_MM

[mbrkusanin]

Summary

Filling a delay slot in 32bit jump instructions with a 16bit instruction can cause issues. According to documentation such an operation is unpredictable. Multiple test from test-suite that fail on microMIPS show this spot as source of failure. This patch adds opcode Mips::PseudoIndirectBranch_MM alongside Mips::PseudoIndirectBranch and other instructions that are expanded to jr instruction and do not allow a 16bit instruction in their delay slots.

[sdardis]

LGTM apart from some minor nits. Please address them before committing.

test/CodeGen/Mips/pseudo-jump-fill.ll

1 ↗

(On Diff #187792)

Drop the '--check-prefixes=MIPS'. Also use the update_llc_checks.py script.

2 ↗

(On Diff #187792)

Add a quick description of the purpose of this test is, i.e. "Test that the delay slot filler correctly handles indirect branches for microMIPS."

5 ↗

(On Diff #187792)

These check lines can be removed, just rely on the update_llc_checks.py script.

16 ↗

(On Diff #187792)

Remove the ' preds = %entry,...' here and below.

[sdardis]

Sorry I didn't spot this earlier, but in future please ensure 'llvm-commits' is one of the subscribers when creating a review request for LLVM. If you add it after creating a review request, manually add it and write something in the comments field to trigger Phabricator into sending an email or abandon the review request and re-open it with the relevant -commits list as an initial subscriber.

Posting review requests without the relevant -commits list means that only the subscribers added, subscribers added through Herald rules and initial reviewers will see the request. It is policy that patches are emailed to the relevant list for review[1]. Submitting patches through Phabricator is fine, provided the relevant -commits list is in the subscribers.

Thanks.

[1] http://www.llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch

[mbrkusanin]

Addressed review comments.

llvm/trunk/lib/Target/Mips/MipsDelaySlotFiller.cpp

for (IterTy I = Begin; I != End;) {
     unsigned Opcode = (*Slot).getOpcode();
     // This is complicated by the tail call optimization. For non-PIC code
     // there is only a 32bit sized unconditional branch which can be assumed
     // to be able to reach the target. b16 only has a range of +/- 1 KB.
     // It's entirely possible that the target function is reachable with b16
     // but we don't have enough information to make that decision.
      if (InMicroMipsMode && TII->getInstSizeInBytes(*CurrI) == 2 &&
         (Opcode == Mips::JR || Opcode == Mips::PseudoIndirectBranch ||
+         Opcode == Mips::PseudoIndirectBranch_MM ||
          Opcode == Mips::PseudoReturn || Opcode == Mips::TAILCALL))
       continue;
      // Instructions LWP/SWP and MOVEP should not be in a delay slot as that
      // results in unpredictable behaviour
      if (InMicroMipsMode && (Opcode == Mips::LWP_MM || Opcode == Mips::SWP_MM ||
                              Opcode == Mips::MOVEP_MM))
        continue;
 

llvm/trunk/test/CodeGen/Mips/pseudo-jump-fill.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=mipsel-linux-gnu -mattr=+micromips -relocation-model=pic < %s | FileCheck %s
+
+; Test that the delay slot filler correctly handles indirect branches for
+; microMIPS in regard to incorrectly using 16bit instructions in delay slots of
+; 32bit instructions.
+
+define i32 @test(i32 signext %x, i32 signext %c) {
+; CHECK-LABEL: test:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    lui $2, %hi(_gp_disp)
+; CHECK-NEXT:    addiu $2, $2, %lo(_gp_disp)
+; CHECK-NEXT:    addiur2 $5, $5, -1
+; CHECK-NEXT:    sltiu $1, $5, 4
+; CHECK-NEXT:    beqz $1, $BB0_3
+; CHECK-NEXT:    addu $3, $2, $25
+; CHECK-NEXT:  $BB0_1: # %entry
+; CHECK-NEXT:    li16 $2, 0
+; CHECK-NEXT:    sll16 $5, $5, 2
+; CHECK-NEXT:    lw $6, %got($JTI0_0)($3)
+; CHECK-NEXT:    addu16 $5, $5, $6
+; CHECK-NEXT:    lw $5, %lo($JTI0_0)($5)
+; CHECK-NEXT:    addu16 $3, $5, $3
+; CHECK-NEXT:    jr $3
+; CHECK-NEXT:    nop
+; CHECK-NEXT:  $BB0_2: # %sw.bb2
+; CHECK-NEXT:    addiur2 $2, $4, 1
+; CHECK-NEXT:    jrc $ra
+; CHECK-NEXT:  $BB0_3:
+; CHECK-NEXT:    move $2, $4
+; CHECK-NEXT:    jrc $ra
+; CHECK-NEXT:  $BB0_4: # %sw.bb3
+; CHECK-NEXT:    addius5 $4, 2
+; CHECK-NEXT:    move $2, $4
+; CHECK-NEXT:    jrc $ra
+; CHECK-NEXT:  $BB0_5: # %sw.bb5
+; CHECK-NEXT:    addius5 $4, 3
+; CHECK-NEXT:    move $2, $4
+; CHECK-NEXT:  $BB0_6: # %for.cond.cleanup
+; CHECK-NEXT:    jrc $ra
+entry:
+  switch i32 %c, label %sw.epilog [
+    i32 4, label %sw.bb5
+    i32 1, label %for.cond.cleanup
+    i32 2, label %sw.bb2
+    i32 3, label %sw.bb3
+  ]
+
+sw.bb2:
+  %add = add nsw i32 %x, 1
+  br label %sw.epilog
+
+sw.bb3:
+  %add4 = add nsw i32 %x, 2
+  br label %sw.epilog
+
+sw.bb5:
+  %add6 = add nsw i32 %x, 3
+  br label %sw.epilog
+
+sw.epilog:
+  %a.0 = phi i32 [ %add6, %sw.bb5 ], [ %add4, %sw.bb3 ], [ %add, %sw.bb2 ], [ %x, %entry ]
+  br label %for.cond.cleanup
+
+for.cond.cleanup:
+  %a.028 = phi i32 [ %a.0, %sw.epilog ], [ 0, %entry ]
+  ret i32 %a.028
+}