Skip to content

Commit

Permalink
[Thumb] set code alignment for 16-bit load from constant pool
Browse files Browse the repository at this point in the history
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
  • Loading branch information
simonwallis2 committed Jul 22, 2020
1 parent 5567c62 commit 94e4e37
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 1 deletion.
6 changes: 5 additions & 1 deletion llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
Expand Up @@ -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
Expand Down
59 changes: 59 additions & 0 deletions 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
...

0 comments on commit 94e4e37

Please sign in to comment.