Skip to content
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

make RUSTFLAGS="-Ccodegen-units=N" always starts building at rsbegin.o #30063

Closed
jonas-schievink opened this issue Nov 25, 2015 · 13 comments
Closed

Comments

@jonas-schievink
Copy link
Contributor

Needless to say, this is really inconvenient. This is happening because the make recipe calls rustc with -o, which is ignored when codegen-units is present. Make will then always recompile rsbegin.o and rsend.o, since they aren't actually created.

The build still works for me, since the files aren't actually needed on Linux, but would probably fail on x86-windows-gnu according to rsbegin.rs (I haven't actually checked).

RUSTFLAGS should probably be ignored for these targets. I have no idea how, since the makefiles look like something straight out of /dev/random.

@petrochenkov
Copy link
Contributor

I've noticed something like this too.
Due to some regression in the build system the rebuild starts from the very beginning (rsbegin.o, rsend.o, libcore) in many cases instead of rebuilding only the necessary crates.
For example, I run make rustc-stage1 -j8 and it successfully finishes, then I modify some crate like rustc, run make rustc-stage1 -j8 again and the rebuild starts with

cp C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/5.2.0/../../../../x86_64-w64-mingw32/lib/../lib/crt2.o x86_64-pc-windows-gnu/rt/crt2.o
cp C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/5.2.0/../../../../x86_64-w64-mingw32/lib/../lib/dllcrt2.o x86_64-pc-windows-gnu/rt/dllcrt2.o
cp: x86_64-pc-windows-gnu/stage1/lib/rustlib/x86_64-pc-windows-gnu/lib/crt2.o
cp: x86_64-pc-windows-gnu/stage1/lib/rustlib/x86_64-pc-windows-gnu/lib/dllcrt2.o
cp: x86_64-pc-windows-gnu/stage0/lib/rustlib/x86_64-pc-windows-gnu/lib/crt2.o
cp: x86_64-pc-windows-gnu/stage0/lib/rustlib/x86_64-pc-windows-gnu/lib/dllcrt2.o
rustc: x86_64-pc-windows-gnu/stage0/lib/rustlib/x86_64-pc-windows-gnu/lib/libcore
rustc: x86_64-pc-windows-gnu/stage0/lib/rustlib/x86_64-pc-windows-gnu/lib/rsbegin.o

I'm on Windows/MSYS2 as you can see.

@alexcrichton
Copy link
Member

This may be because we don't actually emit foo.o when compiling foo.rs with --emit obj, we emit foo.N.o. (could probably disable codegen-units if emit=obj is present)

@pnkfelix
Copy link
Member

pnkfelix commented Dec 4, 2015

@alexcrichton disabling codegen-units in the presence of emit=obj alone seems a little stronger than necessary. Would it suffice to disable codegen-units merely in the presence of -o <path> ?

@pnkfelix
Copy link
Member

pnkfelix commented Dec 4, 2015

Would it suffice to disable codegen-units merely in the presence of -o ?

I guess this would also be stronger than necessary, since as stated it would even cover cases where one is generating a binary to execute (which today is compatible with -C codegen-units=N).

So maybe what I really wanted to ask about was disabling codegen-units in the presence of both -o <path> and emit=obj...

@alexcrichton
Copy link
Member

Well specifically I would expect --emit obj to only emit one object file, and that doesn't happen when there are multiple codegen units (multiple object files are emitted). In that sense to me this is orthogonal of -o, it's just related to the emission of an object.

Note that emission of a binary is fine because we link the objects together into one binary, so it's unaffected by codegen units.

@pnkfelix
Copy link
Member

pnkfelix commented Dec 4, 2015

@alexcrichton that's interesting; I had thought it would at least acceptable (if perhaps unexpected) for --emit obj to emit multiple object files. I.e., the user just wants to control what kind of output is produced, and is willing to go poring over the file system afterwards to find the resulting artifacts.

I'll admit that does not sound like an ideal user interface. But since the workaround is easy when one does want one particular output file (namely to use -o), it seems like a more expressive setup than making --emit obj imply that codegen-units is reset to 1...

By the way, on the subject of selecting file names for outputs, see also #30204

@alexcrichton
Copy link
Member

Hm I guess I just see codegen units as a bit of an optimization (e.g. along the lines of -C opt-level) which shouldn't affect the output artifacts that much. With a GNU linker we can actually take multiple object files and assemble them into one (using the -r flag to ld), but we're unable to do that on MSVC right now (link.exe doesn't support that), so while one day we may be able to support multiple codegen units and one output object file it's unfortunately difficult to do so today.

@pnkfelix
Copy link
Member

pnkfelix commented Dec 4, 2015

@alexcrichton Sure, I am not suggesting that we try today to make multiple codegen units work with one output object file. I'm more saying that we can embrace the semantics we are already providing, where e.g. rustc -C codegen-units=4 --emit=asm means "generate four distinct assembly code files"

@alexcrichton
Copy link
Member

Yeah I guess I'm not totally opposed to that either, but I feel like it makes bugs like this surprising and hard to solve. If you've gone out of your way to say --emit obj you're likely to then later programmatically use that object file later on, and everyone who does this will have to remove codegen units flags or handle multiple object files. Not impossible, but tradeoffs!

@pnkfelix
Copy link
Member

pnkfelix commented Dec 4, 2015

@alexcrichton okay well I'm about to put up a PR to "fix" the narrow case of combining all of -C codegen-units=N_GT_1, --emit=TRANS_TYPE, and -o OUTPUT

So once that's up I'll ask you to make a call as to whether to go with that short-term worse-is-better solution, or if we should hold out for a more principled design.

@alexcrichton
Copy link
Member

Ok, sounds good to me, thanks @pnkfelix!

@pnkfelix
Copy link
Member

pnkfelix commented Dec 4, 2015

(another option could be to add plural variants for each of the emit-types in question, e.g. --emit=obj vs --emit=objs ... --emit=obj would have the semantics suggested by @alexcrichton , where it is guaranteed to produce a single object file and thus will reset codegen-units to 1 if necessary, while the new --emit=objs is explicitly allowed to produce multiple object files, with the numeric suffixes that codegen-units produces today to differentiate them. So we'd add --emit=objs, --emit=asms, et cetera...)

@alexcrichton
Copy link
Member

Yeah that seems reasonable, although I wouldn't want to push forward with that just quite yet.

pnkfelix added a commit to pnkfelix/rust that referenced this issue Dec 8, 2015
…ts back to 1.

Fix rust-lang#30063

Note: while this code is careful to handle the case of mutliple emit
types (e.g. `--emit=asm,obj`) by reporting all the emit types that
conflict with codegen units in its warnings, an invocation with
multiple emit types *and* `-o PATH` will continue to ignore the
requested target path (with a warning), as it already does today,
since the code that checks for that is further downstream.
pnkfelix added a commit to pnkfelix/rust that referenced this issue Dec 8, 2015
pnkfelix added a commit to pnkfelix/rust that referenced this issue Dec 8, 2015
The `rsbegin.o` and `rsend.o` build products should not be generated
on non WinGnu platforms.

This is another path to resolving rust-lang#30063 for non win-gnu targets.
(And it won't require a snapshot, unlike PR rust-lang#30208.)
bors added a commit that referenced this issue Dec 9, 2015
 When given `rustc -C codegen-units=4 --emit=obj`, reset units back to 1.

Fix #30063

Note: while this code is careful to handle the case of mutliple emit types (e.g. `--emit=asm,obj`) by reporting all the emit types that conflict with codegen units in its warnings, an invocation with multiple emit types *and* `-o PATH` will continue to ignore the requested target path (with a warning), as it already does today, since the code that checks for that is further downstream.  (Multiple emit types without `-o PATH` will "work", though it will downgrade codegen-units to 1 just like all the other cases.)

r? @alexcrichton
bors added a commit that referenced this issue Dec 9, 2015
…alexcrichton

The `rsbegin.o` and `rsend.o` build products should not be generated
on non WinGnu platforms.

This is another path to resolving #30063 for non win-gnu targets.
(And it won't require a snapshot, unlike PR #30208.)

r? @alexcrichton
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants