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

Custom target: data-layout changed #31367

Closed
phil-opp opened this Issue Feb 2, 2016 · 10 comments

Comments

Projects
None yet
9 participants
@phil-opp
Copy link
Contributor

phil-opp commented Feb 2, 2016

I use a custom target file to build code for ARM. I copied the file from somewhere and it worked without problems. But now, after an update to the latest nightly, the build fails with a strange LLVM error:

rustc:
/buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/src/llvm/lib/CodeGen/MachineFunction.cpp:108:
llvm::MachineFunction::MachineFunction(const llvm::Function*, const
llvm::TargetMachine&, unsigned int, llvm::MachineModuleInfo&): Assertion
`TM.isCompatibleDataLayout(getDataLayout()) && "Can't create a
MachineFunction using a Module with a " "Target-incompatible DataLayout
attached\n"' failed.

I don't understand any of this but it seems like the data-layout is suddently “Target-incompatible”. My target json looks like this:

{
    "arch": "arm",
    "cpu": "cortex-m4",
    "data-layout": "e-m:e-p:32:32-i1:8:32-i8:8:32-i16:16:32-i64:64-v128:64:128-a:0:32-n32-S64",
    "disable-redzone": true,
    "executables": true,
    "llvm-target": "thumbv7em-none-eabihf",
    "morestack": false,
    "os": "none",
    "relocation-model": "static",
    "target-endian": "little",
    "target-pointer-width": "32",
    "no-compiler-rt": true,
    "pre-link-args": [
        "-mcpu=cortex-m7", "-mthumb",
        "-Tlayout.ld"
    ],
    "post-link-args": [
        "-lm", "-lgcc", "-lnosys"
    ]
}

Removing the data-layout line fixes it and everything works again. So I think this issue is already solved :). I opened it anyway as I wasn't sure if this was intended (and maybe it helps someone).

If someone wants to investigate: The travis builds for nightly libcore started failing on rustc 1.8.0-nightly (9a07087bc 2016-01-30).

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Feb 2, 2016

This might be related to how LLVM was upgraded recently?

@steveklabnik steveklabnik added the A-LLVM label Feb 2, 2016

@dpc

This comment has been minimized.

Copy link
Contributor

dpc commented Feb 3, 2016

This also happend to me on travis where LLVM is in version 3.4. https://travis-ci.org/dpc/titanos/jobs/106659993 when compiling for ARMv8 aarch64 target. It works (well, fails but in unrelated way) on my local machine where llvm is in version 3.7.

Removing data-layout also helped. Is it OK to skip this line?

@tari

This comment has been minimized.

Copy link
Contributor

tari commented Feb 4, 2016

I'm seeing this issue as well, with JSON targets for x86_64 that use a datalayout copied from rustc's x86_64-unknown-linux-gnu target. The LLVM assert does not trigger on the same code when using the built-in linux target.

Trying some nightlies, I still see this crash with the build from January 30 (303892e) and all subsequent ones. Testing further back will require downgrading the copy of libcore I'm building.


Removing datalayout from the target specification prevents the failure, but it's not clear to me what the implications of that are; either rustc or LLVM is quietly choosing a default, where if it's the former everything should be fine and if it's the latter there's a chance of hard-to-debug miscompilation.

ryanbreen added a commit to ryanbreen/breenix that referenced this issue Feb 19, 2016

hannobraun added a commit to hannobraun/embedded that referenced this issue Feb 19, 2016

Work around Rust compiler crash
The issue I'm working around is described here:
rust-lang/rust#31367

I don't see any problems that result from removing the data layout
description, but I'm not at all sure if this is the right thing to do.
If weird, hard-to-debug errors show up at some point, this might be the
source.

rasendubi added a commit to rasendubi/bkernel that referenced this issue Feb 20, 2016

Update to rustc 1.8.0-nightly (57c357d89 2016-02-16)
- data-layout is no longer needed in target file. (Changed in previous
  commit.)
  See rust-lang/rust#31367

- Symbols with custom section should be marked as no_mangle; otherwise,
  LTO will collect them.
  See rust-lang/rust#31758

silven added a commit to silven/armstrong that referenced this issue Feb 22, 2016

bors added a commit that referenced this issue Mar 20, 2016

Auto merge of #32375 - phil-opp:patch-1, r=japaric
docs: The `data-layout` field is no longer required in custom targets

The `data-layout` field is no longer required. It was made optional in 958d563.

The `os` field is always required.

Related to #31367
@thejpster

This comment has been minimized.

Copy link

thejpster commented Apr 22, 2016

So I tried 1.10.0-nightly (b5ba592 2016-04-21) with my bare metal ARM project and it says it "failed to run rustc to learn about target-specific information". In verbose mode it complained that "Field data-layout in target specification is required".

Oh, well, I'll put it back in then. But if you put it back in, you still get this issue.

I note the O/Ps Travis build is also broken, so it's probably not just me.

@phil-opp

This comment has been minimized.

Copy link
Contributor

phil-opp commented Apr 22, 2016

The data-layout field was made manditory again in #32939. The correct values for the built-in target triples can be found in the commit diff. The field layout is specified in the LLVM documentation.

I note the O/Ps Travis build is also broken

Thanks for the hint :).

@japaric

This comment has been minimized.

Copy link
Member

japaric commented Apr 22, 2016

FWIW, I have found two ways to make llvm generate a (seemingly) correct data layout for you:

Using clang

$ touch empty.c
$ clang --target=thumbv7m-none-eabi -S -emit-llvm empty.c -o empty.ll
$ head empty.ll
; ModuleID = 'empty.c'
target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
target triple = "thumbv7m-none--eabi"

Where thumbv7m-none-eabi is the llvm-target field in your target specification file. And clang should be recent-ish, probably 3.8+ or you'll get the old, longer data-layout format.

Using an old rustc

The other way is to use a rustc nightly from 2016-04-20 or older:

$ cat empty.rs
#![feature(no_core)]
#![crate_type = "rlib"]
#![no_core]

$ rustup run nightly-2016-04-20 rustc --target stm32f100 --emit=llvm-ir empty.rs
$ head empty.ll
; ModuleID = 'empty.0.rs'
target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
target triple = "thumbv7m-none--eabi"

Where stm32f100 is your custom target, your target specification file must be in $PWD.

phil-opp added a commit to phil-opp/nightly-libcore that referenced this issue Apr 22, 2016

@thejpster

This comment has been minimized.

Copy link

thejpster commented Apr 23, 2016

I tried the clang method with clang-3.5, but the value it produced did not work. On a whim, I added a:0:32 and then it compiled OK. Haven't actually tested the binary though.

@pravic

This comment has been minimized.

Copy link
Contributor

pravic commented Apr 25, 2016

The other way is to use a rustc nightly from 2016-04-20 or older

You can use stable as well, I guess:

$ touch empty.rs
$ rustup run stable-i686   rustc --emit=llvm-ir --crate-type rlib -o x86.ll empty.rs
$ rustup run stable-x86_64 rustc --emit=llvm-ir --crate-type rlib -o x64.ll empty.rs

hannobraun added a commit to hannobraun/embedded that referenced this issue Apr 26, 2016

Add data layout to target specification
The data layout had become optional at some point. Some time after that,
it started causing a compiler error, so I removed it. From the Rust
side, those changes are documented in the following issue:
rust-lang/rust#31367

This is the pull request that made the data layout non-optional again,
is this one:
rust-lang/rust#32939

I took the layout I added here from the Rust compiler code. The various
built-in ARM targets seem to have mostly[1] the same target layout, which
makes sense, as the target layout describes mostly hardware
characteristics that shouldn't change between operation systems.

The layout I copied is from the `arm-unknown-linux-gnueabi` target,
here:
https://github.com/rust-lang/rust/blob/253b7c1e1a919a6b722c29a04241d6f08ff8c79a/src/librustc_back/target/arm_unknown_linux_gnueabi.rs#L19

I double-checked with the LLVM documentation on data layouts, and
everything seems legit, as far as I can tell. I don't completely
understand everything about it, though, so I can't give a 100%
guarantee. The LLVM documentation on data layouts is located here:
http://llvm.org/docs/LangRef.html#data-layout

In any case, the program compiles and works fine with the new layout, so
I'm assuming it's correct.

[1] "Mostly", because the one exception I see is the name mangline
    option ("m:"), which I set to "e", meaning "ELF". This doesn't seem
    terribly relevant. The only case, that I can think of, that might
    make it relevant is if we had C code calling into our Rust code, but
    then we would mark the called Rust functions as "#[no_mangle]"
    anyway.

thejpster pushed a commit to thejpster/stellaris-launchpad that referenced this issue Apr 26, 2016

hawkw added a commit to sos-os/kernel that referenced this issue May 29, 2016

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member

Mark-Simulacrum commented May 3, 2017

Is there something in particular that is left to do here? I can't quite see any "bug" in this issue, though it would be great to document what @japaric says about getting a seemingly correct data layout somewhere.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Aug 25, 2017

AFAIK there's not much left to do here, so closing.

phil-opp added a commit to phil-opp/rust that referenced this issue Oct 8, 2017

Fix data-layout field
The value was generated according to [this comment by @japaric](rust-lang#31367 (comment)).

kennytm added a commit to kennytm/rust that referenced this issue Oct 10, 2017

Rollup merge of rust-lang#45108 - phil-opp:patch-2, r=japaric
Fix data-layout field in x86_64-unknown-linux-gnu.json test file

The current data-layout causes the following error:

> rustc: /checkout/src/llvm/lib/CodeGen/MachineFunction.cpp:151: void llvm::MachineFunction::init(): Assertion `Target.isCompatibleDataLayout(getDataLayout()) && "Can't create a MachineFunction using a Module with a " "Target-incompatible DataLayout attached\n"' failed.

The new value was generated according to [this comment by @japaric](rust-lang#31367 (comment)).

kennytm added a commit to kennytm/rust that referenced this issue Oct 10, 2017

Rollup merge of rust-lang#45108 - phil-opp:patch-2, r=japaric
Fix data-layout field in x86_64-unknown-linux-gnu.json test file

The current data-layout causes the following error:

> rustc: /checkout/src/llvm/lib/CodeGen/MachineFunction.cpp:151: void llvm::MachineFunction::init(): Assertion `Target.isCompatibleDataLayout(getDataLayout()) && "Can't create a MachineFunction using a Module with a " "Target-incompatible DataLayout attached\n"' failed.

The new value was generated according to [this comment by @japaric](rust-lang#31367 (comment)).

mikhail-m1 pushed a commit to mikhail-m1/rust that referenced this issue Oct 11, 2017

Fix data-layout field
The value was generated according to [this comment by @japaric](rust-lang#31367 (comment)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment