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

Don't necessarily enforce file extension of .bc, .ll, .s files #1869

Merged
merged 1 commit into from
Nov 11, 2016

Conversation

kinke
Copy link
Member

@kinke kinke commented Nov 5, 2016

Use the output filename (-of or inferred) directly if a single file is emitted per compiled module.

Fixes issue #1843.

@thewilsonator
Copy link
Contributor

IIUC you mean if more than one of -output-{bc,ll,s,o} is specified them still force the extensions, but not for object files (I need different extensions for object files, don't change this)?
And you mean llvm modules, not d modules,yes?
I'm just trying to think how this will interoperate with my dcompute PRs which will have multiple files being outputted, but they will each come from one distinct llvm module.

Anyway if this is to be merged please also merge ldc-developers:master into ldc-developers:dcompute, so that I can test properly.

@thewilsonator
Copy link
Contributor

This will have merge conflicts with #1840

@kinke
Copy link
Member Author

kinke commented Nov 6, 2016

IIUC you mean if more than one of -output-{bc,ll,s,o} is specified them still force the extensions, but not for object files

Exactly, as the diff clearly shows. Object filenames aren't touched, only bc/ll/s. The point being that we get a single object filename (-of or inferred) per produced object, so the default .o/.obj extension normally needs to be replaced for bc/ll/s files. But in case only one kind is produced, we may use the filename directly (if inferred, it already has the right .bc/.ll/.s extension), and this is how the explicit -of can make it through the front-end to the name of the single bc/ll/s file being produced.

And you mean llvm modules, not d modules,yes?

Yep, i.e., a single or multiple (-singleobj) D modules compiled to an 'object' (incl. bc/ll/s).

In essence, this only affects the output filename of a bc/ll/s file if (1) a single -output-{bc,ll,s} flag is used, (2) -output-o isn't used (also preventing the generation of a static lib/executable/shared lib, as -of would apply to that name then) and (3) -of is specified (implying -singleobj), i.e., a single 'object' file with that name is to be produced. E.g., ldc2 -output-ll -of=bla.IR foo.d [bar.d].

@kinke kinke mentioned this pull request Nov 7, 2016
@JohanEngelen
Copy link
Member

Is there a way we can test this?
Perhaps %ldc ... -output-s -of=%T/test.A && %remove %T/test.A, relying on failure of %remove when the file is not found?

@kinke
Copy link
Member Author

kinke commented Nov 9, 2016

I added a tiny test, but opted for a textual IR file and an existence check via FileCheck.

Use the output filename (-of or inferred) directly if a single file is
emitted per compiled module.

Fixes issue ldc-developers#1843.
if (global.params.output_bc)
llvm::sys::path::replace_extension(bcpath, global.bc_ext);
std::string bcpath =
(doLTO && outputObj) ? filename : replaceExtensionWith(global.bc_ext);
Copy link
Member Author

Choose a reason for hiding this comment

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

@JohanEngelen: I guess this fixes a potential issue wrt. -flto=<something> -output-o -output-bc, as I guess the regular object filename is required for the bitcode file in the LTO case.

Copy link
Member

Choose a reason for hiding this comment

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

Hm, I think with -output-o -output-bc -flto=.. this codeblock should be executed twice. Two files should be output, right? One for -output-o (bitcode because LTO), and one for output-bc.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's okay not outputting the redundant .bc copy in the LTO case for this exotic cmdline-flags constellation.

@JohanEngelen JohanEngelen merged commit 39a5aae into ldc-developers:master Nov 11, 2016
@JohanEngelen
Copy link
Member

Cool.

@kinke kinke deleted the fix1843 branch November 11, 2016 21:20
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.

3 participants