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

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added inline comments.


================
Comment at: clang/include/clang/Basic/CodeGenOptions.def:52
+                                       ///< Produce unique section names with
+				       ///< basic block sections.
 ENUM_CODEGENOPT(FramePointer, FramePointerKind, 2, FramePointerKind::None) /// frame-pointer: all,non-leaf,none
----------------
Nit: indentation is off here.


================
Comment at: clang/include/clang/Basic/CodeGenOptions.def:341
+CODEGENOPT(RelocateWithSymbols, 1, 0)
+
 /// Whether we should use the undefined behaviour optimization for control flow
----------------
Can you add a doc here? (possibly referring to somewhere else if it is extensively documented elsewhere?)


================
Comment at: clang/include/clang/Driver/Options.td:1873
+def fbasicblock_sections_EQ : Joined<["-"], "fbasicblock-sections=">, Group<f_Group>, Flags<[CC1Option, CC1AsOption]>,
+  HelpText<"Place each function's basic blocks in unique sections (ELF Only) : all | labels | none | <filename>">;
 def fdata_sections : Flag <["-"], "fdata-sections">, Group<f_Group>,
----------------
Is the "labels" options dependent/related to the previous -fpropeller-label one?


================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:348
+        Twine NewName = Section + Name;
+        Fn->setSection(NewName.str());
+      }
----------------
Is this related to these new option? It this change the behavior of -ffunction-sections?


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1032
     }
-
+  }
   return Out.str();
----------------
I agree with the improvement, but as nit this isn't related to the current patch or even in a function you're otherwise touching. (it creates an extra hunk in the review)


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1100
+  // internal linkage functions, to differentiate the symbols across
+  // modules.
+  if (getCodeGenOpts().UniqueInternalFuncNames &&
----------------
What happens in case of conflict? (maybe clarify in the comment)


Repository:
  rC Clang

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

https://reviews.llvm.org/D68049