Skip to content

Commit ea222be

Browse files
lenarylenary
authored
[MC] Honour alignment directive fill value for non-intel (#100136)
As reported in https://llvm.org/PR30955, `.balign` with a fill-value of 0 did not actually align using zeroes, on non-x86 targets. This is because the check of whether to use the code alignment routines or whether to just use the fill value was checking whether the fill value was equal to `TextAlignFillValue`, which has not been changed from its default of 0 on most targets (it has been changed for x86). However, most targets do not set the fill value because it doesn't entirely make sense -- i.e. on AArch64 there's no reasonable byte value to use for alignment, as instructions are word-sized and have to be well-aligned. I think the check at the end `AsmParser::parseDirectiveAlign` is suspicious even on x86 - if you use `.balign <align>, 0x90` in a code section, you don't end up with a block of `0x90` repeated, you end up with a block of NOPs of various widths. This functionality is never tested. The fix here is to modify the check to ignore the default text align fill value when choosing to do code alignment or not. Fixes #30303
1 parent bb0300c commit ea222be

File tree

14 files changed

+149
-41
lines changed

14 files changed

+149
-41
lines changed

‎lld/test/COFF/lto-cpu-string.ll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ target triple = "x86_64-pc-windows-msvc19.14.26433"
1414

1515
define dllexport void @foo() #0 {
1616
entry:
17-
call void asm sideeffect ".p2align 4, 0x90", "~{dirflag},~{fpsr},~{flags}"()
17+
call void asm sideeffect ".p2align 4", "~{dirflag},~{fpsr},~{flags}"()
1818
ret void
1919
}
2020

‎lld/test/ELF/lto/cpu-string.ll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ target triple = "x86_64-unknown-linux-gnu"
1818

1919
define void @foo() #0 {
2020
entry:
21-
call void asm sideeffect ".p2align 4, 0x90", "~{dirflag},~{fpsr},~{flags}"()
21+
call void asm sideeffect ".p2align 4", "~{dirflag},~{fpsr},~{flags}"()
2222
ret void
2323
}
2424

‎lld/test/ELF/lto/mllvm.ll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16
1717

1818
define void @_start() #0 {
1919
entry:
20-
call void asm sideeffect ".p2align 4, 0x90", "~{dirflag},~{fpsr},~{flags}"()
20+
call void asm sideeffect ".p2align 4", "~{dirflag},~{fpsr},~{flags}"()
2121
ret void
2222
}
2323

‎lld/test/MachO/lto-cpu-string.ll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16
1616

1717
define void @foo() #0 {
1818
entry:
19-
call void asm sideeffect ".p2align 4, 0x90", "~{dirflag},~{fpsr},~{flags}"()
19+
call void asm sideeffect ".p2align 4", "~{dirflag},~{fpsr},~{flags}"()
2020
ret void
2121
}
2222

‎llvm/docs/ReleaseNotes.rst‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,20 @@ Changes to Interprocedural Optimizations
6565
Changes to the AArch64 Backend
6666
------------------------------
6767

68+
* `.balign N, 0`, `.p2align N, 0`, `.align N, 0` in code sections will now fill
69+
the required alignment space with a sequence of `0x0` bytes (the requested
70+
fill value) rather than NOPs.
71+
6872
Changes to the AMDGPU Backend
6973
-----------------------------
7074

7175
Changes to the ARM Backend
7276
--------------------------
7377

78+
* `.balign N, 0`, `.p2align N, 0`, `.align N, 0` in code sections will now fill
79+
the required alignment space with a sequence of `0x0` bytes (the requested
80+
fill value) rather than NOPs.
81+
7482
Changes to the AVR Backend
7583
--------------------------
7684

@@ -92,6 +100,10 @@ Changes to the PowerPC Backend
92100
Changes to the RISC-V Backend
93101
-----------------------------
94102

103+
* `.balign N, 0`, `.p2align N, 0`, `.align N, 0` in code sections will now fill
104+
the required alignment space with a sequence of `0x0` bytes (the requested
105+
fill value) rather than NOPs.
106+
95107
Changes to the WebAssembly Backend
96108
----------------------------------
97109

@@ -101,6 +113,13 @@ Changes to the Windows Target
101113
Changes to the X86 Backend
102114
--------------------------
103115

116+
* `.balign N, 0x90`, `.p2align N, 0x90`, and `.align N, 0x90` in code sections
117+
now fill the required alignment space with repeating `0x90` bytes, rather than
118+
using optimised NOP filling. Optimised NOP filling fills the space with NOP
119+
instructions of various widths, not just those that use the `0x90` byte
120+
encoding. To use optimised NOP filling in a code section, leave off the
121+
"fillval" argument, i.e. `.balign N`, `.p2align N` or `.align N` respectively.
122+
104123
Changes to the OCaml bindings
105124
-----------------------------
106125

‎llvm/lib/MC/MCParser/AsmParser.cpp‎

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3462,17 +3462,6 @@ bool AsmParser::parseDirectiveAlign(bool IsPow2, unsigned ValueSize) {
34623462
}
34633463
}
34643464

3465-
if (HasFillExpr && FillExpr != 0) {
3466-
MCSection *Sec = getStreamer().getCurrentSectionOnly();
3467-
if (Sec && Sec->isVirtualSection()) {
3468-
ReturnVal |=
3469-
Warning(FillExprLoc, "ignoring non-zero fill value in " +
3470-
Sec->getVirtualSectionKind() + " section '" +
3471-
Sec->getName() + "'");
3472-
FillExpr = 0;
3473-
}
3474-
}
3475-
34763465
// Diagnose non-sensical max bytes to align.
34773466
if (MaxBytesLoc.isValid()) {
34783467
if (MaxBytesToFill < 1) {
@@ -3489,13 +3478,20 @@ bool AsmParser::parseDirectiveAlign(bool IsPow2, unsigned ValueSize) {
34893478
}
34903479
}
34913480

3492-
// Check whether we should use optimal code alignment for this .align
3493-
// directive.
34943481
const MCSection *Section = getStreamer().getCurrentSectionOnly();
34953482
assert(Section && "must have section to emit alignment");
3496-
bool useCodeAlign = Section->useCodeAlign();
3497-
if ((!HasFillExpr || Lexer.getMAI().getTextAlignFillValue() == FillExpr) &&
3498-
ValueSize == 1 && useCodeAlign) {
3483+
3484+
if (HasFillExpr && FillExpr != 0 && Section->isVirtualSection()) {
3485+
ReturnVal |=
3486+
Warning(FillExprLoc, "ignoring non-zero fill value in " +
3487+
Section->getVirtualSectionKind() +
3488+
" section '" + Section->getName() + "'");
3489+
FillExpr = 0;
3490+
}
3491+
3492+
// Check whether we should use optimal code alignment for this .align
3493+
// directive.
3494+
if (Section->useCodeAlign() && !HasFillExpr) {
34993495
getStreamer().emitCodeAlignment(
35003496
Align(Alignment), &getTargetParser().getSTI(), MaxBytesToFill);
35013497
} else {

‎llvm/test/MC/AArch64/align-fill-byte-zero.s‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// RUN: llvm-mc -triple aarch64 %s -o - | FileCheck %s --check-prefix=ASM
2+
// RUN: llvm-mc -triple aarch64 -filetype obj %s -o - | \
3+
// RUN: llvm-objdump -dz - | FileCheck %s --check-prefix=OBJ
4+
5+
// llvm.org/pr30955 - LLVM was handling `.balign <alignment>, 0` strangely on
6+
// non-x86 targets.
7+
8+
.text
9+
10+
// ASM: add x14, x14, #1
11+
// OBJ: 910005ce add x14, x14, #0x1
12+
add x14, x14, 0x1
13+
14+
// ASM: .p2align 4, 0x0
15+
// OBJ-NEXT: 00000000 udf #0x0
16+
// OBJ-NEXT: 00000000 udf #0x0
17+
// OBJ-NEXT: 00000000 udf #0x0
18+
.balign 0x10, 0
19+
20+
// ASM: add x14, x14, #1
21+
// OBJ-NEXT: 910005ce add x14, x14, #0x1
22+
add x14, x14, 0x1

‎llvm/test/MC/ARM/align-fill-byte-zero.s‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// RUN: llvm-mc -triple armv7a %s -o - | FileCheck %s --check-prefix=ASM-ARM
2+
// RUN: llvm-mc -triple armv7a -filetype obj %s -o - | \
3+
// RUN: llvm-objdump --triple=armv7a -dz - | FileCheck %s --check-prefix=OBJ-ARM
4+
5+
// RUN: llvm-mc -triple thumbv7a %s -o - | FileCheck %s --check-prefix=ASM-THUMB
6+
// RUN: llvm-mc -triple thumbv7a -filetype obj %s -o - | \
7+
// RUN: llvm-objdump --triple=thumbv7a -dz - | FileCheck %s --check-prefix=OBJ-THUMB
8+
9+
// llvm.org/pr30955 - LLVM was handling `.balign <alignment>, 0` strangely on
10+
// non-x86 targets.
11+
12+
.text
13+
14+
// ASM-ARM: add r0, r0, #1
15+
// OBJ-ARM: e2800001 add r0, r0, #1
16+
17+
// ASM-THUMB: add.w r0, r0, #1
18+
// OBJ-THUMB: f100 0001 add.w r0, r0, #0x1
19+
add r0, r0, 0x1
20+
21+
// ASM-ARM: .p2align 4, 0x0
22+
// OBJ-ARM-NEXT: 00000000 andeq r0, r0, r0
23+
// OBJ-ARM-NEXT: 00000000 andeq r0, r0, r0
24+
// OBJ-ARM-NEXT: 00000000 andeq r0, r0, r0
25+
26+
// ASM-THUMB: .p2align 4, 0x0
27+
// OBJ-THUMB-NEXT: 0000 movs r0, r0
28+
// OBJ-THUMB-NEXT: 0000 movs r0, r0
29+
// OBJ-THUMB-NEXT: 0000 movs r0, r0
30+
// OBJ-THUMB-NEXT: 0000 movs r0, r0
31+
// OBJ-THUMB-NEXT: 0000 movs r0, r0
32+
// OBJ-THUMB-NEXT: 0000 movs r0, r0
33+
.balign 0x10, 0
34+
35+
// ASM-ARM: add r0, r0, #1
36+
// OBJ-ARM-NEXT: e2800001 add r0, r0, #1
37+
38+
// ASM-THUMB: add.w r0, r0, #1
39+
// OBJ-THUMB-NEXT: f100 0001 add.w r0, r0, #0x1
40+
add r0, r0, 0x1

‎llvm/test/MC/AsmParser/directive_align.s‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
# RUN: not llvm-mc -triple i386-apple-darwin9 %s 2> %t.err | FileCheck %s
22
# RUN: FileCheck < %t.err %s --check-prefix=CHECK-WARN
33

4+
.data
5+
46
# CHECK: TEST0:
57
# CHECK: .p2align 1
68
TEST0:

‎llvm/test/MC/COFF/align-nops.s‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
.text
55
f0:
66
.long 0
7-
.align 8, 0x90
7+
.align 8
88
.long 0
99
.align 8
1010

0 commit comments

Comments
 (0)