[ARM] FP16 constant pool fix

[SjoerdMeijer]

Summary

This is a follow up of r325012, that allowed half types in constant pools.
Proper alignment was enforced when a big basic block was split up, but not when
a CPE was placed before/after a block; the successor block had the wrong
alignment.

[olista01]

LGTM, but please wait a day to let other time-zones comment.

[SjoerdMeijer]

Thanks for reviewing!

lib/Target/ARM/ARMConstantIslandPass.cpp

if (CC != ARMCC::AL)
       MI = LastIT;
   }
 
   // We really must not split an IT block.
   DEBUG(unsigned PredReg;
         assert(!isThumb || getITInstrPredicate(*MI, PredReg) == ARMCC::AL));
 
   NewMBB = splitBlockBeforeInstr(&*MI);
-
-  // 4 byte align the next block after the constant pool when the CPE is a
-  // 16-bit value in ARM mode, and 2 byte for Thumb.
-  NewMBB->setAlignment(isThumb ? 1 : 2);
 }
 
 /// handleConstantPoolUser - Analyze the specified user, checking to see if it
 /// is out-of-range.  If so, pick up the constant pool value and move it some
 /// place in-range.  Return true if we changed any addresses (thus must run
 /// another pass of branch lengthening), false otherwise.
 bool ARMConstantIslands::handleConstantPoolUser(unsigned CPUserIndex,
                                                 bool CloserWater) {
if (findAvailableWater(U, UserOffset, IP, CloserWater)) {
     MachineBasicBlock *WaterBB = &*--NewMBB->getIterator();
     IP = find(WaterList, WaterBB);
     if (IP != WaterList.end())
       NewWaterList.erase(WaterBB);
 
     // We are adding new water.  Update NewWaterList.
     NewWaterList.insert(NewIsland);
   }
+  // Always align the new block because CP entries can be smaller than 4 bytes.
+  NewMBB->setAlignment(isThumb ? 1 : 2);
 
   // Remove the original WaterList entry; we want subsequent insertions in
   // this vicinity to go after the one we're about to insert.  This
   // considerably reduces the number of times we have to move the same CPE
   // more than once and is also important to ensure the algorithm terminates.
   if (IP != WaterList.end())
     WaterList.erase(IP);
 

test/CodeGen/ARM/constant-islands-cfg.mir

frameInfo:
   savePoint:       ''
   restorePoint:    ''
 fixedStack:
 # CHECK-LABEL: name: test_split_cfg
 # CHECK: bb.0:
 # CHECK:     successors: %[[LONG_BR_BB:bb.[0-9]+]](0x{{[0-9a-f]+}}), %[[DEST1:bb.[0-9]+]](0x{{[0-9a-f]+}}){{$}}
 # CHECK:     tBcc %[[LONG_BR_BB]], 0, $cpsr
 # CHECK:     tB %[[DEST1]]
-# CHECK: [[LONG_BR_BB]]:
+# CHECK: [[LONG_BR_BB]] (align 1):
 # CHECK:     successors: %[[DEST2:bb.[0-9]+]](0x{{[0-9a-f]+}}){{$}}
 # CHECK:     tB %[[DEST2]]
 # CHECK: [[DEST1]]:
 # CHECK: [[DEST2]]:
 
 body:             |
   bb.0:
     liveins: $r0
     tCMPi8 killed $r0, 0, 14, $noreg, implicit-def $cpsr
     tBcc %bb.2, 1, killed $cpsr
     tB %bb.3, 14, $noreg
 
-  bb.1:
+  bb.1 (align 1):
     dead $r0 = SPACE 256, undef $r0
 
   bb.2:
     tPOP_RET 14, $noreg, def $pc
 
   bb.3:
     tPOP_RET 14, $noreg, def $pc
 
 ...

test/CodeGen/ARM/fp16-litpool-arm.mir

 # RUN: llc -mtriple=arm-none-eabi -run-pass=arm-cp-islands %s -o - | FileCheck %s
-
+#
+# This checks alignment of a new block when a big basic block is split up.
+#
 --- |
   ; ModuleID = '<stdin>'
   source_filename = "<stdin>"
   target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
   target triple = "arm-arm--eabi"
 
   declare i32 @llvm.arm.space(i32, i32) #0
 

test/CodeGen/ARM/fp16-litpool-thumb.mir

 # RUN: llc -mtriple=thumbv7-none-eabi -run-pass=arm-cp-islands %s -o - | FileCheck %s
+#
+# This checks alignment of a new block when a big basic block is split up.
+#
 --- |
   ; ModuleID = '<stdin>'
   source_filename = "<stdin>"
   target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
   target triple = "thumbv7-arm--eabi"
 
   declare i32 @llvm.arm.space(i32, i32) #0
 

test/CodeGen/ARM/fp16-litpool2-arm.mir

+# RUN: llc -mtriple=arm-none-eabi -run-pass=arm-cp-islands %s -o - | FileCheck %s
+#
+# This checks alignment of a block when a CPE is placed before/after a
+# block (as e.g. opposed to splitting up a block).
+#
+--- |
+  ; ModuleID = '<stdin>'
+  source_filename = "<stdin>"
+  target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+  target triple = "arm-arm--eabi"
+
+  declare i32 @llvm.arm.space(i32, i32) #0
+
+  define dso_local i32 @CP() #1 {
+  entry:
+    %res = alloca half, align 2
+    store half 0xH706B, half* %res, align 2
+    %0 = load half, half* %res, align 2
+    %tobool = fcmp une half %0, 0xH0000
+    br i1 %tobool, label %LA, label %END
+
+  LA:                                               ; preds = %entry
+    %1 = call i32 @llvm.arm.space(i32 1000, i32 undef)
+    br label %END
+
+  END:                                              ; preds = %LA, %entry
+    %2 = call i32 @llvm.arm.space(i32 100, i32 undef)
+    ret i32 42
+  }
+
+  ; Function Attrs: nounwind
+  declare void @llvm.stackprotector(i8*, i8**) #2
+
+  attributes #0 = { nounwind "target-features"="+v8.2a,+fullfp16" }
+  attributes #1 = { "target-features"="+v8.2a,+fullfp16" }
+  attributes #2 = { nounwind }
+
+...
+---
+name:            CP
+alignment:       2
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+registers:
+liveins:
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       4
+  offsetAdjustment: 0
+  maxAlignment:    2
+  adjustsStack:    false
+  hasCalls:        false
+  stackProtector:  ''
+  maxCallFrameSize: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:
+stack:
+  - { id: 0, name: res, type: default, offset: -2, size: 2, alignment: 2,
+      stack-id: 0, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: -2, di-variable: '', di-expression: '', di-location: '' }
+constants:
+  - id:              0
+    value:           half 0xH706B
+    alignment:       2
+    isTargetSpecific: false
+
+
+#CHECK:  bb.{{.*}} (align 1):
+#CHECK:    successors:
+#CHECK:    CONSTPOOL_ENTRY 1, %const{{.*}}, 2
+# We want this block to be 4 byte aligned:
+#CHECK:  bb.{{.*}}.LA (align 2):
+
+body:             |
+  bb.0.entry:
+    successors: %bb.1(0x50000000), %bb.2(0x30000000)
+
+    $sp = frame-setup SUBri $sp, 4, 14, $noreg, $noreg
+    frame-setup CFI_INSTRUCTION def_cfa_offset 4
+    renamable $s0 = VLDRH %const.0, 0, 14, $noreg :: (load 2 from constant-pool)
+    VCMPZH renamable $s0, 14, $noreg, implicit-def $fpscr_nzcv
+    VSTRH killed renamable $s0, $sp, 1, 14, $noreg :: (store 2 into %ir.res)
+    FMSTAT 14, $noreg, implicit-def $cpsr, implicit killed $fpscr_nzcv
+    Bcc %bb.2, 0, killed $cpsr
+
+  bb.1.LA:
+    successors: %bb.2(0x80000000)
+
+    dead renamable $r0 = SPACE 1000, undef renamable $r0
+
+  bb.2.END:
+    dead renamable $r0 = SPACE 100, undef renamable $r0
+    $r0 = MOVi 42, 14, $noreg, $noreg
+    $sp = ADDri $sp, 4, 14, $noreg, $noreg
+    BX_RET 14, $noreg, implicit killed $r0
+
+...