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

[Sriraman Tallam via Phabricator via llvm-commits]

tmsriram marked 2 inline comments as done.
tmsriram added a comment.

In D68063#1913547 <https://reviews.llvm.org/D68063#1913547>, @echristo wrote:

> Also the commit message is awesome, but would be good to get the commit message represented as comments in lots of the final code if possible :)


Will do, I can go over the code and add these comments in appropriate places but if you have specific locations please mention.



================
Comment at: llvm/include/llvm/Target/TargetOptions.h:71-72
+    All,    // Use Basic Block Sections for all functions.
+    Labels, // Do not use Basic Block Sections but label basic blocks.
+    List    // Get list of functions & BBs from a file
+  };
----------------
echristo wrote:
> Can you describe the use case behind this in the comments?
Sure.  Just to be clear, are you referring only to "List" or everything?


================
Comment at: llvm/include/llvm/Target/TargetOptions.h:275-277
+    /// Memory Buffer that contains information on sampled basic blocks and used
+    /// to selectively generate basic block sections.
+    std::shared_ptr<MemoryBuffer> BBSectionsFuncListBuf;
----------------
echristo wrote:
> Why would you want to be able to copy it again?
I am not copying Options but this is existing code.  I can dig exactly where but Options is being copied and that disallows unique_ptr.  shared_ptr seems fine as I only want one copy of the MemoryBuffer and that is guaranteed.


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

https://reviews.llvm.org/D68063