[llvm] 94e4e37 - [Thumb] set code alignment for 16-bit load from constant pool

[Simon Wallis via llvm-commits]

Author: Simon Wallis
Date: 2020-07-22T10:12:41+01:00
New Revision: 94e4e37d5564c401dcdc7f98d36d87020b6759e1

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

LOG: [Thumb] set code alignment for 16-bit load from constant pool

Summary:
[Thumb] set code alignment for 16-bit load from constant pool

LLVM miscompiles this code when compiling for a target with v8.2-A FP16 and the Thumb ISA at -O0:

extern void bar(__fp16 P5);

int main() {
  __fp16 P5 = 1.96875;
  bar(P5);
}

The code section containing main has 2 byte alignment.
It needs to have 4 byte alignment,
because the load literal instruction has an offset from the
load address with the low 2 bits zeroed.

I do not include a test case in this check-in.
llc and llvm-mc do not exhibit this bug. They do not set code section alignment
in the same manner as clang.

Reviewers: dnsampaio

Reviewed By: dnsampaio

Subscribers: hiraditya, llvm-commits

Tags: #llvm

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

Added: 
    llvm/test/CodeGen/ARM/const-load-align-thumb.mir

Modified: 
    llvm/lib/Target/ARM/ARMConstantIslandPass.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/ARM/ARMConstantIslandPass.cpp b/llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
index 195d0a89291b..204e57fefb9a 100644
--- a/llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
+++ b/llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
@@ -491,7 +491,11 @@ ARMConstantIslands::doInitialConstPlacement(std::vector<MachineInstr*> &CPEMIs)
 
   // The function needs to be as aligned as the basic blocks. The linker may
   // move functions around based on their alignment.
-  MF->ensureAlignment(BB->getAlignment());
+  // Special case: halfword literals still need word alignment on the function.
+  Align FuncAlign = MaxAlign;
+  if (MaxAlign == 2)
+    FuncAlign = Align(4);
+  MF->ensureAlignment(FuncAlign);
 
   // Order the entries in BB by descending alignment.  That ensures correct
   // alignment of all entries as long as BB is sufficiently aligned.  Keep

diff  --git a/llvm/test/CodeGen/ARM/const-load-align-thumb.mir b/llvm/test/CodeGen/ARM/const-load-align-thumb.mir
new file mode 100644
index 000000000000..daa8a9103309
--- /dev/null
+++ b/llvm/test/CodeGen/ARM/const-load-align-thumb.mir
@@ -0,0 +1,59 @@
+# RUN: llc -mtriple=arm-eabi -run-pass=arm-cp-islands %s -o - | FileCheck %s
+--- |
+  target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+  target triple = "thumbv8.2a-arm-none-eabi"
+
+  define hidden i32 @main() {
+  entry:
+    %P5 = alloca half, align 2
+    store half 0xH3FE0, half* %P5, align 2
+    %0 = load half, half* %P5, align 2
+    call void @z_bar(half %0)
+    ret i32 0
+  }
+
+  declare dso_local void @z_bar(half)
+
+...
+---
+name:            main
+alignment:       2
+tracksRegLiveness: true
+frameInfo:
+  stackSize:       16
+  maxAlignment:    4
+  adjustsStack:    true
+  hasCalls:        true
+  maxCallFrameSize: 0
+  localFrameSize:  2
+stack:
+  - { id: 0, name: P5, offset: -10, size: 2, alignment: 2, local-offset: -2 }
+  - { id: 1, type: spill-slot, offset: -4, size: 4, alignment: 4, callee-saved-register: '$lr',
+      callee-saved-restored: false }
+  - { id: 2, type: spill-slot, offset: -8, size: 4, alignment: 4, callee-saved-register: '$r7' }
+constants:
+  - id:              0
+    value:           half 0xH3FE0
+    alignment:       2
+machineFunctionInfo: {}
+body:             |
+  bb.0.entry:
+    liveins: $r7, $lr
+
+    frame-setup tPUSH 14 /* CC::al */, $noreg, killed $r7, 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 $r7, -8
+    $sp = frame-setup tSUBspi $sp, 2, 14 /* CC::al */, $noreg
+    frame-setup CFI_INSTRUCTION def_cfa_offset 16
+    renamable $s0 = VLDRH %const.0, 0, 14, $noreg :: (load 2 from constant-pool)
+    VSTRH killed renamable $s0, $sp, 3, 14, $noreg :: (store 2 into %ir.P5)
+    renamable $r0 = t2LDRHi12 $sp, 6, 14 /* CC::al */, $noreg :: (dereferenceable load 2 from %ir.P5)
+    tBL 14 /* CC::al */, $noreg, @z_bar, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $r0, implicit-def $sp
+    renamable $r0, dead $cpsr = tMOVi8 0, 14 /* CC::al */, $noreg
+    $sp = frame-destroy tADDspi $sp, 2, 14 /* CC::al */, $noreg
+    frame-destroy tPOP_RET 14 /* CC::al */, $noreg, def $r7, def $pc, implicit killed $r0
+
+  ; CHECK: name:            main
+  ; CHECK-NEXT: alignment:       4
+...