[clang] Correctly link and optimize device libraries with -mlink-builtin-bitcode (PR #69371)

Jacob Lambert via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 6 14:18:02 PST 2023


lamb-j wrote:

> this doesn't really apply since you changed the function signature so it needs to be reformatted.

I don't follow the logic there. The function signature can have a style as well. And I think this is actually a good example to demonstrate a reason not to reformat if we look at the next few lines:

Following existing style:
```
  void RunOptimizationPipeline(BackendAction Action,
                          std::unique_ptr<raw_pwrite_stream> &OS,
                          std::unique_ptr<llvm::ToolOutputFile> &ThinLinkOS,
                          BackendConsumer *BC);
  void RunCodegenPipeline(BackendAction Action,
                          std::unique_ptr<raw_pwrite_stream> &OS,
                          std::unique_ptr<llvm::ToolOutputFile> &DwoOS);
```


Following clang format:
```
  void RunOptimizationPipeline(
      BackendAction Action, std::unique_ptr<raw_pwrite_stream> &OS,
      std::unique_ptr<llvm::ToolOutputFile> &ThinLinkOS,
      BackendConsumer *BC);
  void RunCodegenPipeline(BackendAction Action,
                          std::unique_ptr<raw_pwrite_stream> &OS,
                          std::unique_ptr<llvm::ToolOutputFile> &DwoOS);
```

To me, the first case aligns more with the spirit of the "golden rule". And if someone wants to reformat the whole file at a later point, that would be a good time to change the style for both signatures.

That said, I definitely don't want this to be a barrier to getting this patch in, so if you still feel like we should go with the clang-format recommendation, I'll change it and also update the EmitAssembly and EmitBackendOutput signatures which were flagged by clang-format for the same reasons.

https://github.com/llvm/llvm-project/pull/69371


More information about the cfe-commits mailing list