[PATCH] D68063: Propeller: LLVM support for basic block sections

[Sriraman Tallam via Phabricator via llvm-commits]

tmsriram added a comment.

In D68063#1913667 <https://reviews.llvm.org/D68063#1913667>, @MaskRay wrote:

> > To be clear I think this is close to being acceptable
>
> Agreed. I am still waiting for the resolution to my (very old) comment about:


Then, let me end your wait right away! :)

> 
> 
>   namespace BasicBlockSection {
>     enum SectionMode {
> 
> 
> A scoped enumeration (`enum class`) will be clearer.

I put it in its own namespace like many other enums in that file which are not scoped.  If you insist, I can remove the namespace and make it scoped.

> The summary is very long and includes lots of stuff not touched by this patch. The relevant paragraphs should be moved to a subsequent patch I think.

IMO, The ideal place to put the summary would be in BBSectionsPrepare.cpp which is the pass that does the actual analysis.  I will do that.


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

https://reviews.llvm.org/D68063