[llvm] b91f239 - MipsDelaySlotFiller: Don't move BUNDLE instructions into the delay slot

Alex Richardson via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 4 03:31:20 PST 2019


Author: Alex Richardson
Date: 2019-12-04T11:30:00Z
New Revision: b91f239485fb7bb8d29be3e0b60660a2de7570a9

URL: https://github.com/llvm/llvm-project/commit/b91f239485fb7bb8d29be3e0b60660a2de7570a9
DIFF: https://github.com/llvm/llvm-project/commit/b91f239485fb7bb8d29be3e0b60660a2de7570a9.diff

LOG: MipsDelaySlotFiller: Don't move BUNDLE instructions into the delay slot

Summary:
In our CHERI fork we use BUNDLE instructions to ensure that a
three-instruction sequence to generate a program-counter-relative value is
emitted without reordering or insertions (since that would break the 32-bit
offset computation). This sequence is created in MipsExpandPseudo and we use
finalizeBundle() to create the BUNDLE instruction.

However, the delay slot filler currently breaks this pattern since the BUNDLE
will be removed and so all instructions are moved into the delay slot.
Since the delay slot only executes the first instruction, this results in
incorrect computations (and run-time crashes) if the branch is taken.

The original test cases uses CHERI instructions, so for the test case here
I simple filled a BUNDLE with a no-op DADDiu $sp_64, -16 and DADDiu $sp_64, 16.

Reviewers: atanasyan

Reviewed By: atanasyan

Subscribers: merge_guards_bot, sdardis, hiraditya, jrtc27, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D70944

Added: 
    llvm/test/CodeGen/Mips/delay-slot-filler-bundled-insts.mir

Modified: 
    llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp b/llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp
index 4a1f9ac11027..60d14933a2e0 100644
--- a/llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp
+++ b/llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp
@@ -695,6 +695,12 @@ bool MipsDelaySlotFiller::searchRange(MachineBasicBlock &MBB, IterTy Begin,
       continue;
     }
 
+    if (CurrI->isBundle()) {
+      LLVM_DEBUG(dbgs() << DEBUG_TYPE ": ignoring BUNDLE instruction: ";
+                 CurrI->dump());
+      continue;
+    }
+
     if (terminateSearch(*CurrI)) {
       LLVM_DEBUG(dbgs() << DEBUG_TYPE ": should terminate search: ";
                  CurrI->dump());

diff  --git a/llvm/test/CodeGen/Mips/delay-slot-filler-bundled-insts.mir b/llvm/test/CodeGen/Mips/delay-slot-filler-bundled-insts.mir
new file mode 100644
index 000000000000..07e25da70489
--- /dev/null
+++ b/llvm/test/CodeGen/Mips/delay-slot-filler-bundled-insts.mir
@@ -0,0 +1,142 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+## Check that the delay-slot filler does not attempt to split BUNDLE instructions
+# RUN: llc %s -start-before=mips-delay-slot-filler -stop-after=mips-delay-slot-filler \
+# RUN:   -verify-machineinstrs -o - | FileCheck %s
+--- |
+  target datalayout = "E-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128"
+  target triple = "mips64-unknown-freebsd"
+  declare i8* @func_a(i64 zeroext)
+  declare i8* @func_b(i64 zeroext)
+  ; Function Attrs: nounwind
+  define i8* @test(i64 zeroext %nbytes) local_unnamed_addr #0 {
+  entry:
+    %cmp = icmp eq i64 %nbytes, 0
+    br i1 %cmp, label %if.else, label %if.then
+
+  if.then:                                          ; preds = %entry
+    %call = tail call i8* @func_a(i64 zeroext %nbytes)
+    br label %return
+
+  if.else:                                          ; preds = %entry
+    %call1 = tail call i8* @func_b(i64 zeroext 0)
+    br label %return
+
+  return:                                           ; preds = %if.else, %if.then
+    %retval.0 = phi i8* [ %call, %if.then ], [ %call1, %if.else ]
+    ret i8* %retval.0
+  }
+  ; Function Attrs: nounwind
+  declare void @llvm.stackprotector(i8*, i8**) #0
+
+  attributes #0 = { nounwind }
+
+...
+---
+name:            test
+alignment:       8
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+failedISel:      false
+tracksRegLiveness: true
+hasWinCFI:       false
+registers:       []
+liveins:
+  - { reg: '$a0_64', virtual-reg: '' }
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       16
+  offsetAdjustment: 0
+  maxAlignment:    8
+  adjustsStack:    true
+  hasCalls:        true
+  stackProtector:  ''
+  maxCallFrameSize: 0
+  cvBytesOfCalleeSavedRegisters: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  localFrameSize:  0
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:      []
+stack:
+  - { id: 0, name: '', type: spill-slot, offset: -8, size: 8, alignment: 8,
+      stack-id: default, callee-saved-register: '$ra_64', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+callSites:       []
+constants:       []
+machineFunctionInfo: {}
+body:             |
+  ; CHECK-LABEL: name: test
+  ; CHECK: bb.0.entry:
+  ; CHECK:   successors: %bb.2(0x30000000), %bb.1(0x50000000)
+  ; CHECK:   $sp_64 = DADDiu $sp_64, -16
+  ; CHECK:   CFI_INSTRUCTION def_cfa_offset 16
+  ; CHECK:   SD killed $ra_64, $sp_64, 8 :: (store 8 into %stack.0)
+  ; CHECK:   CFI_INSTRUCTION offset $ra_64, -8
+  ; CHECK:   BUNDLE {
+  ; CHECK:     $sp_64 = DADDiu $sp_64, -16
+  ; CHECK:     $sp_64 = DADDiu $sp_64, 16
+  ; CHECK:   }
+  ; CHECK:   BEQ64 renamable $a0_64, $zero_64, %bb.2, implicit-def $at {
+  ; CHECK:     NOP
+  ; CHECK:   }
+  ; CHECK: bb.1.if.then:
+  ; CHECK:   successors: %bb.3(0x80000000)
+  ; CHECK:   JAL @func_a, csr_n64, implicit-def dead $ra, implicit $a0_64, implicit-def $sp, implicit-def $v0_64 {
+  ; CHECK:     NOP
+  ; CHECK:   }
+  ; CHECK:   J %bb.3, implicit-def dead $at {
+  ; CHECK:     NOP
+  ; CHECK:   }
+  ; CHECK: bb.2.if.else:
+  ; CHECK:   successors: %bb.3(0x80000000)
+  ; CHECK:   JAL @func_b, csr_n64, implicit-def dead $ra, implicit $a0_64, implicit-def $sp, implicit-def $v0_64 {
+  ; CHECK:     $a0_64 = DADDiu $zero_64, 0
+  ; CHECK:   }
+  ; CHECK: bb.3.return:
+  ; CHECK:   $ra_64 = LD $sp_64, 8 :: (load 8 from %stack.0)
+  ; CHECK:   PseudoReturn64 undef $ra_64, implicit $v0_64 {
+  ; CHECK:     $sp_64 = DADDiu $sp_64, 16
+  ; CHECK:   }
+  bb.0.entry:
+    successors: %bb.2(0x30000000), %bb.1(0x50000000)
+    liveins: $a0_64, $ra_64
+
+    $sp_64 = DADDiu $sp_64, -16
+    CFI_INSTRUCTION def_cfa_offset 16
+    SD killed $ra_64, $sp_64, 8 :: (store 8 into %stack.0)
+    CFI_INSTRUCTION offset $ra_64, -8
+    ; This BUNDLE instruction must not be split by the delay slot filler:
+    BUNDLE {
+      $sp_64 = DADDiu $sp_64, -16
+      $sp_64 = DADDiu $sp_64, 16
+    }
+    BEQ64 renamable $a0_64, $zero_64, %bb.2, implicit-def $at
+
+  bb.1.if.then:
+    successors: %bb.3(0x80000000)
+    liveins: $a0_64
+
+    JAL @func_a, csr_n64, implicit-def dead $ra, implicit $a0_64, implicit-def $sp, implicit-def $v0_64
+    J %bb.3, implicit-def dead $at
+
+  bb.2.if.else:
+    successors: %bb.3(0x80000000)
+
+    $a0_64 = DADDiu $zero_64, 0
+    JAL @func_b, csr_n64, implicit-def dead $ra, implicit $a0_64, implicit-def $sp, implicit-def $v0_64
+
+  bb.3.return:
+    liveins: $v0_64
+
+    $ra_64 = LD $sp_64, 8 :: (load 8 from %stack.0)
+    $sp_64 = DADDiu $sp_64, 16
+    PseudoReturn64 undef $ra_64, implicit $v0_64
+
+...


        


More information about the llvm-commits mailing list