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

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

In D68049#1870094 <https://reviews.llvm.org/D68049#1870094>, @tmsriram wrote:

> In D68049#1868623 <https://reviews.llvm.org/D68049#1868623>, @MaskRay wrote:
>
> > > 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..
>


This should work now.  Please apply D68063 <https://reviews.llvm.org/D68063> first and then this one. Thanks!

> Weird, getBBSectionsList is defined by D68063 <https://reviews.llvm.org/D68063>.  Let me try doing that again.  I will also address the rest of your comments.
> 
>> 
>> 
>>   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