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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
