[llvm] 646ca7d - MipsDelaySlotFiller: Update registers def-uses for BUNDLE instructions

[Alex Richardson via llvm-commits]

Author: Alex Richardson
Date: 2020-01-09T20:46:02Z
New Revision: 646ca7d7e72e8408b3fa3472018eb9d1c2643ff5

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

LOG: MipsDelaySlotFiller: Update registers def-uses for BUNDLE instructions

Summary:
In commit b91f239485fb7bb8d29be3e0b60660a2de7570a9 I updated the
MipsDelaySlotFiller to skip BUNDLE instructions.
However, in addition to not considering BUNDLE instructions for the delay
slot, we also need to ensure that the register def-use information is
updated. Not updating this information caused run-time crashes (when using
the out-of-tree CHERI backend) since later definitions could be overwritten
with earlier register values.

Reviewers: atanasyan
Reviewed By: atanasyan
Differential Revision: https://reviews.llvm.org/D72254

Added: 
    llvm/test/CodeGen/Mips/delay-slot-filler-bundled-insts-def-use.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 60d14933a2e0..84ff674569cd 100644
--- a/llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp
+++ b/llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp
@@ -416,8 +416,14 @@ bool RegDefsUses::update(const MachineInstr &MI, unsigned Begin, unsigned End) {
   for (unsigned I = Begin; I != End; ++I) {
     const MachineOperand &MO = MI.getOperand(I);
 
-    if (MO.isReg() && MO.getReg())
-      HasHazard |= checkRegDefsUses(NewDefs, NewUses, MO.getReg(), MO.isDef());
+    if (MO.isReg() && MO.getReg()) {
+      if (checkRegDefsUses(NewDefs, NewUses, MO.getReg(), MO.isDef())) {
+        LLVM_DEBUG(dbgs() << DEBUG_TYPE ": found register hazard for operand "
+                          << I << ": ";
+                   MO.dump());
+        HasHazard = true;
+      }
+    }
   }
 
   Defs |= NewDefs;
@@ -698,6 +704,8 @@ bool MipsDelaySlotFiller::searchRange(MachineBasicBlock &MBB, IterTy Begin,
     if (CurrI->isBundle()) {
       LLVM_DEBUG(dbgs() << DEBUG_TYPE ": ignoring BUNDLE instruction: ";
                  CurrI->dump());
+      // However, we still need to update the register def-use information.
+      RegDU.update(*CurrI, 0, CurrI->getNumOperands());
       continue;
     }
 

diff  --git a/llvm/test/CodeGen/Mips/delay-slot-filler-bundled-insts-def-use.mir b/llvm/test/CodeGen/Mips/delay-slot-filler-bundled-insts-def-use.mir
new file mode 100644
index 000000000000..fcbbfc1b5b5c
--- /dev/null
+++ b/llvm/test/CodeGen/Mips/delay-slot-filler-bundled-insts-def-use.mir
@@ -0,0 +1,104 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+## Check that the delay-slot filler looks at the registers defined by BUNDLE instructions
+# RUN: llc %s -run-pass=mips-delay-slot-filler -verify-machineinstrs -o - | FileCheck %s
+--- |
+  ; ModuleID = '/Users/alex/cheri/llvm-project/llvm/test/CodeGen/Mips/delay-test.ll'
+  source_filename = "/Users/alex/cheri/llvm-project/llvm/test/CodeGen/Mips/delay-test.ll"
+  target datalayout = "E-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128"
+  target triple = "mips64-unknown-freebsd"
+
+  ; Function Attrs: nounwind
+  define i64 @test(i1 %cond) local_unnamed_addr #0 {
+  entry:
+    br i1 %cond, label %err, label %return
+
+  err:                                              ; preds = %entry
+    unreachable
+
+  return:                                           ; preds = %entry
+    ret i64 1
+  }
+
+  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:       0
+  offsetAdjustment: 0
+  maxAlignment:    1
+  adjustsStack:    false
+  hasCalls:        false
+  stackProtector:  ''
+  maxCallFrameSize: 0
+  cvBytesOfCalleeSavedRegisters: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  localFrameSize:  0
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:      []
+stack:           []
+callSites:       []
+constants:       []
+machineFunctionInfo: {}
+body:             |
+  ; CHECK-LABEL: name: test
+  ; CHECK: bb.0.entry:
+  ; CHECK:   successors: %bb.2(0x00000001), %bb.1(0x7fffffff)
+  ; CHECK:   renamable $at = SLL renamable $a0, 0, implicit killed $a0_64
+  ; CHECK:   renamable $at = ANDi killed renamable $at, 1
+  ; CHECK:   $v0_64 = DADDiu $zero_64, 1
+  ; CHECK:   $v0_64 = DADDiu $v0_64, 1
+  ; CHECK:   BUNDLE implicit-def $v0_64 {
+  ; CHECK:     $v0_64 = DADDiu $zero_64, 1
+  ; CHECK:     $v0_64 = DADDiu $v0_64, 1
+  ; CHECK:   }
+  ; CHECK:   BNE killed renamable $at, $zero, %bb.2, implicit-def $at {
+  ; CHECK:     NOP
+  ; CHECK:   }
+  ; CHECK: bb.1.return:
+  ; CHECK:   PseudoReturn64 undef $ra_64, implicit killed $v0_64 {
+  ; CHECK:     NOP
+  ; CHECK:   }
+  ; CHECK: bb.2.err:
+  bb.0.entry:
+    successors: %bb.1(0x00000001), %bb.2(0x7fffffff)
+    liveins: $a0_64
+
+    renamable $at = SLL renamable $a0, 0, implicit killed $a0_64
+    renamable $at = ANDi killed renamable $at, 1
+    ; Check that none of these instructions are hoisted after the BUNDLE to avoid
+    ; incorrect values from being
+    $v0_64 = DADDiu $zero_64, 1
+    $v0_64 = DADDiu $v0_64, 1
+    BUNDLE implicit-def $v0_64 {
+      $v0_64 = DADDiu $zero_64, 1
+      $v0_64 = DADDiu $v0_64, 1
+    }
+    BNE killed renamable $at, $zero, %bb.1, implicit-def $at
+
+  bb.2.return:
+    liveins: $v0_64
+    PseudoReturn64 undef $ra_64, implicit killed $v0_64
+
+  bb.1.err:
+
+...