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

Add support for outputting multiple modules, one per entry point #551

Merged
merged 3 commits into from
Mar 29, 2021

Conversation

khyperia
Copy link
Contributor

Fixes #539

Not sure if I should actually include the examples/multibuilder crate, I'm fine with deleting it (also for some reason it only works when cd'ing into the dir and running cargo run --release, it fails to find the cargo exe when running with cargo run --release -p multibuilder)

@khyperia khyperia requested review from eddyb, fu5ha and VZout as code owners March 29, 2021 11:47
@hrydgard hrydgard changed the title Add support for outputting multiple modes, one per entry point Add support for outputting multiple modules, one per entry point Mar 29, 2021
@khyperia
Copy link
Contributor Author

thanks, my brain is mush right now :P

Copy link
Member

@VZout VZout left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets keep the example. in the future we might want to use a toggle for that in the existing examples tho 🤷

@@ -380,6 +380,9 @@ impl CodegenBackend for SpirvCodegenBackend {
) -> Result<(), ErrorReported> {
// TODO: Can we merge this sym with the one in symbols.rs?
let legalize = !sess.target_features.contains(&Symbol::intern("kernel"));
let emit_multiple_modules = sess
.target_features
.contains(&Symbol::intern("multimodule"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be more appropriate if we accepted this as an argument from -Cllvm-args(sess.cg.llvm_args)? It's not really something that's specific to the the target as it is specific to our backend. Another benefit is since it is just Vec<String>, we could use clap here for getting these arguments, that's make it to have key value arguments like --module-output=[multiple|single].

@khyperia khyperia requested a review from XAMPPRocky March 29, 2021 14:43
@XAMPPRocky XAMPPRocky merged commit c3a3b20 into main Mar 29, 2021
@XAMPPRocky XAMPPRocky deleted the multimodule branch March 29, 2021 15:59
XAMPPRocky pushed a commit that referenced this pull request May 3, 2021
* Add multimodule feature

* Use -Cllvm-args instead of -Ctarget-feature for multimodule

* Fix cargo.toml
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

Successfully merging this pull request may close these issues.

Add option to output multiple files, one per entry point
3 participants