[PATCH] D68049: Propeller: Clang options for basic block sections

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

> In D68049#1865967 <https://reviews.llvm.org/D68049#1865967>, @MaskRay wrote:
>  If you don't mind, I can push a Diff to this Differential which will address these review comments.

I can't because I can't figure out the patch relationship...

First, this patch does not build on its own. I try applying D68063 <https://reviews.llvm.org/D68063> first, then this patch. It still does not compile..

  clang/lib/CodeGen/BackendUtil.cpp:484:11: error: no member named 'propeller' in namespace 'llvm'
      llvm::propeller::getBBSectionsList(CodeGenOpts.BBSections,

Chatted with shenhan and xur offline. I tend to agree that -fbasicblock-sections=label can improve profile accuracy. It'd be nice to make that feature separate,
even if there is still a debate on whether the rest of Propeller is done in a maintainable way.

I think the patch series should probably be structured this way:

1. LLVM CodeGen: enables basic block sections.
2. clang Driver/Frontend/CodeGen: pass basic block sections options to LLVM.

3. LLVM CodeGen: which enables the rest of Propeller options.
4. lld: a file similar to lld/ELF/LTO.cpp . It should be a thin wrapper of Propeller features. It should not do hacky diassembly tasks.
5. clang Driver/Frontend/CodeGen: passes compiler/linker options to 3 and 4

Making 1 and 2 separate can help move forward the patch series. 1 and 2 should not reference `llvm::propeller`.


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

https://reviews.llvm.org/D68049