[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

David Blaikie via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 20 15:04:53 PDT 2022


dblaikie added a comment.

>> (I'd still sort of lean towards "make it the same as the .o, but with the .pcm suffix" and if the build system wants to put things in other places, it can move them around - but I understand why that's not something everyone's on board with)
>
> Actually, I would think that to be a great default (and put it in the CWD just like the object),
> .. but it seems (in addition to the default) we do need a way to place specific module files - and I could imagine needing to make different places for different command line option sets.  Asking the driver or build system to move the file seems like spawning more processes and we'd still need a user-visible way to say where we wanted the file to be put.

At least I'd like these to be separate patches - adding ".cppm -> {.pcm + .o}" action and then separately adding a way to specify the name of the .pcm output file.

But I still have some reservations about even providing a flag for it - I think the example I was thinking of was coverage files ( https://gcc.gnu.org/onlinedocs/gcc/Gcov-Data-Files.html ) which also don't have a custom path option, like Split DWARF. I'd be happy not to add this until someone comes back with a demonstrated need - it doesn't seem like it'd be a major imposition on a build system to copy the file if it needed to to move it into a release tree V the build tree, for instance. And if it did add up to real performance problems, we could add it.

>> Yeah, if the `-fmodule-file` syntax is currently only for input I don't think we need to try to overload it for output as well.
>
> My understanding [which could be flawed] is that the second syntax (which already exists, it's not an invention of this discussion) is intended to allow specification of output files.

I haven't seen evidence of that, but it's certainly not something I've got lots of info on. It looks like it was added in https://reviews.llvm.org/D35020 as a search path.

I think the first patch (be it using this review or another) should be just the ".cppm -> {.pcm + .o}" (which should basically be entirely in the driver, right? Since for now it'll still be two actions that are already implemented) and then another for "here's a flag to name the .pcm output file" (not sure if we need that, really, just yet) and possibly another for "here's an option to write the .pcm file out to a cache directory instead of the .o path or custom .pcm output path" (I've even stronger reservations about that). I guess discussing the second and third somewhat together, if their flag interface might interact (since they both specify where the .pcm file goes).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134267/new/

https://reviews.llvm.org/D134267



More information about the cfe-commits mailing list