Tighten ARMConstantInslands pass for Thumb-1 targets

[rmaprath]

Summary

This pass takes blocks of the form:

%R0<def>, %CPSR<def,dead> = tLSLri %R0<kill>, 2
%R1<def> = tLEApcrelJT <jt#0>
%R0<def> = tLDRr %R0<kill>, %R1<kill>
tBR_JTr %R0<kill>, <jt#0>

And turns them into:

tTBB_JT %PC, %R0<kill>, <jt#0>, 0
JUMPTABLE_TBB 0, <jt#0>, <some-calculated-offset>

However, this assumes that R0 (index register) is not written to between
the tLDRr and the tBR_JTr instructions. This patch tightens the pass so
that when such an intervening instruction is present, the input block is
left un-altered.

[john.brawn]

lib/Target/ARM/ARMConstantIslandPass.cpp

2135

Please use spaces instead of tabs.

[rmaprath]

Addressing review comments: Get rid of the tab characters.

[rmaprath]

Abandoning: @jmolloy has committed a semantically equivalent patch: https://reviews.llvm.org/rL294949

lib/Target/ARM/ARMConstantIslandPass.cpp

       MachineInstr *Load = User.MI->getNextNode();
       if (Load->getOpcode() != ARM::tLDRr)
         continue;
       if (Load->getOperand(1).getReg() != ShiftedIdxReg ||
           Load->getOperand(2).getReg() != BaseReg ||
           !Load->getOperand(1).isKill())
         continue;
 
+      MachineInstr *Next = Load->getNextNode();
+
       // If we're in PIC mode, there should be another ADD following.
       if (isPositionIndependentOrROPI) {
-        MachineInstr *Add = Load->getNextNode();
+        MachineInstr *Add = Next;
+        Next = Add->getNextNode();
         if (Add->getOpcode() != ARM::tADDrr ||
             Add->getOperand(2).getReg() != Load->getOperand(0).getReg() ||
             Add->getOperand(3).getReg() != BaseReg ||
             !Add->getOperand(2).isKill())
           continue;
         if (Add->getOperand(0).getReg() != MI->getOperand(0).getReg())
           continue;
 
         Add->eraseFromParent();
         DeadSize += 2;
       } else {
         if (Load->getOperand(0).getReg() != MI->getOperand(0).getReg())
           continue;
       }
+
+      // If the index register gets written to by an intervening
+      // instruction (after the load/add), we cannot do this dance.
+      bool ClobbersShiftedIdxReg = false;
+      while (Next != nullptr && !ClobbersShiftedIdxReg) {
+        if (Next->definesRegister(ShiftedIdxReg, STI->getRegisterInfo()))
+          ClobbersShiftedIdxReg = true;
+        Next = Next->getNextNode();
+      }
+      if (ClobbersShiftedIdxReg) continue;
       
       // Now safe to delete the load and lsl. The LEA will be removed later.
       CanDeleteLEA = true;
       Shift->eraseFromParent();
       Load->eraseFromParent();
       DeadSize += 4;
     }
     
     DEBUG(dbgs() << "Shrink JT: " << *MI);
     MachineInstr *CPEMI = User.CPEMI;
     unsigned Opc = ByteOk ? ARM::t2TBB_JT : ARM::t2TBH_JT;
     if (!isThumb2)
       Opc = ByteOk ? ARM::tTBB_JT : ARM::tTBH_JT;
 
     MachineBasicBlock::iterator MI_JT = MI;
     MachineInstr *NewJTMI =

test/CodeGen/Thumb/no-cp-islands-opt.mir

+# RUN: llc -run-pass=arm-cp-islands %s -o - | FileCheck %s
+
+# CHECK: tLEApcrelJT
+# CHECK: tBR_JTr
+# CHECK: JUMPTABLE_ADDRS 0
+
+--- |
+  ; ModuleID = 'func.ll'
+  source_filename = "func.ll"
+  target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+  target triple = "thumbv6m-arm-none-eabi"
+  
+  define linkonce_odr hidden i8* @foo(i8* %first, i8* %last) {
+  entry:
+    %0 = load i8, i8* %first, align 1
+    %1 = zext i8 %0 to i32
+    switch i32 %1, label %cleanup [
+      i32 76, label %sw.bb
+      i32 100, label %sw.bb1
+      i32 105, label %sw.bb4
+      i32 113, label %sw.bb4
+    ]
+  
+  sw.bb:                                            ; preds = %entry
+    br label %sw.bb1
+  
+  sw.bb1:                                           ; preds = %sw.bb, %entry
+    %first.addr.0 = phi i8* [ %first, %entry ], [ null, %sw.bb ]
+    %arrayidx = getelementptr inbounds i8, i8* %first, i32 1
+    %2 = load i8, i8* %arrayidx, align 1
+    %cond = icmp eq i8 %2, 108
+    br i1 %cond, label %cleanup, label %sw.bb4
+  
+  sw.bb4:                                           ; preds = %sw.bb1, %entry, %entry
+    %call = tail call i8* @foo(i8* null, i8* %last)
+    ret i8* null
+  
+  cleanup:                                          ; preds = %sw.bb1, %entry
+    %retval.0 = phi i8* [ %first.addr.0, %sw.bb1 ], [ null, %entry ]
+    ret i8* %retval.0
+  }
+  
+
+...
+---
+name:            foo
+alignment:       1
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+liveins:         
+  - { reg: '%r0' }
+  - { reg: '%r1' }
+calleeSavedRegisters: [ '%lr', '%d8', '%d9', '%d10', '%d11', '%d12', '%d13', 
+                        '%d14', '%d15', '%q4', '%q5', '%q6', '%q7', '%r4', 
+                        '%r5', '%r6', '%r7', '%r8', '%r9', '%r10', '%r11', 
+                        '%s16', '%s17', '%s18', '%s19', '%s20', '%s21', 
+                        '%s22', '%s23', '%s24', '%s25', '%s26', '%s27', 
+                        '%s28', '%s29', '%s30', '%s31', '%d8_d10', '%d9_d11', 
+                        '%d10_d12', '%d11_d13', '%d12_d14', '%d13_d15', 
+                        '%q4_q5', '%q5_q6', '%q6_q7', '%q4_q5_q6_q7', '%r4_r5', 
+                        '%r6_r7', '%r8_r9', '%r10_r11', '%d8_d9_d10', '%d9_d10_d11', 
+                        '%d10_d11_d12', '%d11_d12_d13', '%d12_d13_d14', 
+                        '%d13_d14_d15', '%d8_d10_d12', '%d9_d11_d13', '%d10_d12_d14', 
+                        '%d11_d13_d15', '%d8_d10_d12_d14', '%d9_d11_d13_d15', 
+                        '%d9_d10', '%d11_d12', '%d13_d14', '%d9_d10_d11_d12', 
+                        '%d11_d12_d13_d14' ]
+frameInfo:       
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       8
+  offsetAdjustment: 0
+  maxAlignment:    4
+  adjustsStack:    true
+  hasCalls:        true
+  maxCallFrameSize: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+stack:           
+  - { id: 0, type: spill-slot, offset: -4, size: 4, alignment: 4, callee-saved-register: '%lr' }
+  - { id: 1, type: spill-slot, offset: -8, size: 4, alignment: 4, callee-saved-register: '%r4' }
+jumpTable:       
+  kind:            inline
+  entries:         
+    - id:              0
+      blocks:          [ '%bb.2.sw.bb', '%bb.5.cleanup', '%bb.5.cleanup', 
+                         '%bb.5.cleanup', '%bb.5.cleanup', '%bb.5.cleanup', 
+                         '%bb.5.cleanup', '%bb.5.cleanup', '%bb.5.cleanup', 
+                         '%bb.5.cleanup', '%bb.5.cleanup', '%bb.5.cleanup', 
+                         '%bb.5.cleanup', '%bb.5.cleanup', '%bb.5.cleanup', 
+                         '%bb.5.cleanup', '%bb.5.cleanup', '%bb.5.cleanup', 
+                         '%bb.5.cleanup', '%bb.5.cleanup', '%bb.5.cleanup', 
+                         '%bb.5.cleanup', '%bb.5.cleanup', '%bb.5.cleanup', 
+                         '%bb.3.sw.bb1', '%bb.5.cleanup', '%bb.5.cleanup', 
+                         '%bb.5.cleanup', '%bb.5.cleanup', '%bb.4.sw.bb4', 
+                         '%bb.5.cleanup', '%bb.5.cleanup', '%bb.5.cleanup', 
+                         '%bb.5.cleanup', '%bb.5.cleanup', '%bb.5.cleanup', 
+                         '%bb.5.cleanup', '%bb.4.sw.bb4' ]
+body:             |
+  bb.0.entry:
+    successors: %bb.5.cleanup, %bb.1.entry
+    liveins: %r0, %r1, %r4, %lr
+  
+    frame-setup tPUSH 14, _, killed %r4, killed %lr, implicit-def %sp, implicit %sp
+    frame-setup CFI_INSTRUCTION def_cfa_offset 8
+    frame-setup CFI_INSTRUCTION offset %lr, -4
+    frame-setup CFI_INSTRUCTION offset %r4, -8
+    %r3 = tLDRBi %r0, 0, 14, _ :: (load 1 from %ir.first)
+    %r3, dead %cpsr = tSUBi8 killed %r3, 76, 14, _
+    %r2, dead %cpsr = tMOVi8 0, 14, _
+    tCMPi8 %r3, 37, 14, _, implicit-def %cpsr
+    tBcc %bb.5.cleanup, 8, killed %cpsr
+  
+  bb.1.entry:
+    successors: %bb.2.sw.bb, %bb.5.cleanup, %bb.3.sw.bb1, %bb.4.sw.bb4
+    liveins: %r0, %r1, %r2, %r3
+  
+    %r3, dead %cpsr = tLSLri killed %r3, 2, 14, _
+    %r4 = tLEApcrelJT %jump-table.0, 14, _
+    %r4 = tLDRr killed %r3, killed %r4, 14, _ :: (load 4 from jump-table)
+    %r3 = tMOVr %r0, 14, _
+    tBR_JTr killed %r4, %jump-table.0
+  
+  bb.2.sw.bb:
+    successors: %bb.3.sw.bb1
+    liveins: %r0, %r1
+  
+    %r3, dead %cpsr = tMOVi8 0, 14, _
+  
+  bb.3.sw.bb1:
+    successors: %bb.5.cleanup, %bb.4.sw.bb4
+    liveins: %r0, %r1, %r3
+  
+    %r0 = tLDRBi killed %r0, 1, 14, _ :: (load 1 from %ir.arrayidx)
+    tCMPi8 killed %r0, 108, 14, _, implicit-def %cpsr
+    %r2 = tMOVr killed %r3, 14, _
+    tBcc %bb.4.sw.bb4, 1, killed %cpsr
+  
+  bb.5.cleanup:
+    liveins: %r2
+  
+    %r0 = tMOVr killed %r2, 14, _
+    tPOP_RET 14, _, def %r4, def %pc, implicit-def %sp, implicit %sp, implicit %r0
+  
+  bb.4.sw.bb4:
+    liveins: %r1
+  
+    %r4, dead %cpsr = tMOVi8 0, 14, _
+    %r0 = tMOVr %r4, 14, _
+    tBL 14, _, @foo, csr_aapcs, implicit-def dead %lr, implicit %sp, implicit %r0, implicit %r1, implicit-def %sp, implicit-def dead %r0
+    %r0 = tMOVr killed %r4, 14, _
+    tPOP_RET 14, _, def %r4, def %pc, implicit-def %sp, implicit %sp, implicit %r0
+
+...