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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:964
+        << Opts.BBSections;
+  }
+
----------------
grimar wrote:
> Doesn't seem you need "{}" around this lines. (Its a single call and looks inconsistent with the code around).
> (The same for the change above)
Please mark this comment as done.


================
Comment at: lld/ELF/LTO.cpp:79
   // Check if basic block sections must be used.
   // Allowed values for --lto-basicblock-sections are "all", "labels",
   // "<file name specifying basic block ids>", or none.  This is the equivalent
----------------
tmsriram wrote:
> MaskRay wrote:
> > `--lto-basic-block-sections`
> > 
> > I think it should be fine updating it here, and probably preferable, since you already touched some related LLD code. It does not affect any tests. (You did not add any tests for `--lto-basicblock-sections` in the original LLD patch.)
> Yes, I intend to clean this up and the llc option in a separate cleanup patch so that this doesn't get too complicated.  Is that alright?
It is ok.


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

https://reviews.llvm.org/D68049