[PATCH] D144596: Add extra parameter to thinlto-prefix-replace

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D144596#4186838 <https://reviews.llvm.org/D144596#4186838>, @tejohnson wrote:

> In D144596#4182584 <https://reviews.llvm.org/D144596#4182584>, @MaskRay wrote:
>
>> While we're here, let's make two more changes. First, let's use `.bc` instead of `.o` for the bitcode file extension name.
>
> I'm not in favor of hardcoding the extension in the compiler, it should be left to the build system, but I don't believe that what you are proposing would require a specific extension anyway. Currently, the object output file extension of the compile action is not a hardcoded requirement and I don't think it should be. Additionally, when we were initially implementing ThinLTO I recall that the Bazel team specifically wanted to keep the .o extension (otherwise the handling gets a bit trickier for them as a compile action will have different extensions depending on the optimization options). But like I said it doesn't appear to me that what you are proposing would require this.

My proposal is about the convention used in Bazel. I don't propose to hard code the `.bc` path in llvm-project.
Switching from `.o` to `.bc` in Bazel is for clarity.

>> Second, let's place the initially compiled bitcode files in `thin/` instead of the current working directory for clarity.
>
> Again, this should be decided in the build system, not by the compiler. But it also isn't clear to me that your proposal requires a specific name, just that your proposal is that we require the full and minimized bitcode to be in separate directories and supply the mapping between those instead of the mapping between the extensions. As mentioned above, I would like to understand from @itf if that is needed for tree artifacts, and discuss this in a separate patch, as it seems more like a refinement and less like a requirement, and is orthogonal to this particular change. It may also require additional changes to the build system.

`thin/` is for an arbitrary directory for exposition purpose.
It's just a convention in my example to show how distributed ThinLTO will work.
The Bazel implementation may prefer something like `_objs*` or other directories.


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

https://reviews.llvm.org/D144596