Skip to content

s390x needs minimum alignment for global variables #44411

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
cuviper opened this issue Sep 8, 2017 · 1 comment
Closed

s390x needs minimum alignment for global variables #44411

cuviper opened this issue Sep 8, 2017 · 1 comment
Labels
A-codegen Area: Code generation C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@cuviper
Copy link
Member

cuviper commented Sep 8, 2017

Clang has TargetInfo::MinGlobalAlign which sets a minimum alignment for global variables (unless explicitly aligned less). This is needed for s390x to have 2-byte alignment even for 1-byte-aligned types, as explained in clang r181210:

This patch adds a new common code feature that allows platform code to
request minimum alignment of global symbols.  The background for this is
that on SystemZ, the most efficient way to load addresses of global symbol
is the LOAD ADDRESS RELATIVE LONG (LARL) instruction.  This instruction
provides PC-relative addressing, but only to *even* addresses.  For this
reason, existing compilers will guarantee that global symbols are always
aligned to at least 2.  [ Since symbols would otherwise already use a
default alignment based on their type, this will usually only affect global
objects of character type or character arrays. ]  GCC also allows creating
symbols without that extra alignment by using explicit "aligned" attributes
(which then need to be used on both definition and each use of the symbol).

For Rust, I ran into trouble here when running bootstrap from 1.21-beta. It now uses serde_json, and the static serde_json::read::ESCAPE: [bool; 256] was misaligned. In the binary and in memory it was at an odd address, but the generated code/relocation tried to read it from an even address, so it was off by one. This led to serde_json throwing an InvalidUnicodeCodePoint error on a space (0x20), because it was effectively indexing the entry for 0x1F (which needs to be escaped in JSON).

@cuviper
Copy link
Member Author

cuviper commented Sep 8, 2017

For comparison, here is Clang's LLVM-IR:

$ cat foo.c
#include <stdbool.h>
const bool FOO = true;

$ clang --target=s390x-unknown-linux-gnu -S -emit-llvm -std=c11 foo.c -o -
; ModuleID = 'foo.c'
source_filename = "foo.c"
target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-a:8:16-n32:64"
target triple = "s390x-unknown-linux-gnu"

@FOO = constant i8 1, align 2

!llvm.ident = !{!0}

!0 = !{!"clang version 4.0.1 (tags/RELEASE_401/final)"}

And here is Rust's LLVM-IR:

$ cat foo.rs
pub static FOO: bool = true;

$ rustc --target=s390x-unknown-linux-gnu --crate-type=rlib --emit=llvm-ir foo.rs

$ cat foo.ll
; ModuleID = 'foo.cgu-0.rs'
source_filename = "foo.cgu-0.rs"
target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-a:8:16-n32:64"
target triple = "s390x-unknown-linux-gnu"

@_ZN3foo3FOO17h5b2ce89c23349598E = constant i8 1, align 1

@alexcrichton alexcrichton added A-codegen Area: Code generation C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 8, 2017
bors added a commit that referenced this issue Sep 11, 2017
Add `TargetOptions::min_global_align`, with s390x at 16-bit

The SystemZ `LALR` instruction provides PC-relative addressing for globals,
but only to *even* addresses, so other compilers make sure that such
globals are always 2-byte aligned.  In Clang, this is modeled with
`TargetInfo::MinGlobalAlign`, and `TargetOptions::min_global_align` now
serves the same purpose for rustc.

In Clang, the only targets that set this are SystemZ, Lanai, and NVPTX, and
the latter two don't have targets in rust master.

Fixes #44411.
r? @eddyb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants