[Remarks] Emit the remarks section by default for certain formats

Emit a remarks section by default for the following formats:

* bitstream
* yaml-strtab

while still providing -remarks-section=<bool> to override the defaults.

Author: francisvm

llvm/docs/Remarks.rst

@@ -581,9 +581,13 @@ This diff file can be displayed using :ref:`opt-viewer.py <optviewerpy>`.
 Emitting remark diagnostics in the object file
 ==============================================
 
-A section containing metadata on remark diagnostics will be emitted when
--remarks-section is passed. The section contains the metadata associated to the
-format used to serialize the remarks.
+A section containing metadata on remark diagnostics will be emitted for the
+following formats:
+
+* ``yaml-strtab``
+* ``bitstream``
+
+This can be overridden by using the flag ``-remarks-section=<bool>``.
 
 The section is named:
 

llvm/include/llvm/CodeGen/AsmPrinter.h

@@ -70,6 +70,7 @@ class MCTargetOptions;
 class MDNode;
 class Module;
 class raw_ostream;
+class RemarkStreamer;
 class StackMaps;
 class TargetLoweringObjectFile;
 class TargetMachine;
@@ -319,7 +320,7 @@ class AsmPrinter : public MachineFunctionPass {
 
   void emitStackSizeSection(const MachineFunction &MF);
 
-  void emitRemarksSection(Module &M);
+  void emitRemarksSection(RemarkStreamer &RS);
 
   enum CFIMoveType { CFI_M_None, CFI_M_EH, CFI_M_Debug };
   CFIMoveType needsCFIMoves() const;

llvm/include/llvm/IR/RemarkStreamer.h

@@ -53,6 +53,8 @@ class RemarkStreamer {
   Error setFilter(StringRef Filter);
   /// Emit a diagnostic through the streamer.
   void emit(const DiagnosticInfoOptimizationBase &Diag);
+  /// Check if the remarks also need to have associated metadata in a section.
+  bool needsSection() const;
 };
 
 template <typename ThisError>

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

@@ -146,11 +146,6 @@ static const char *const CodeViewLineTablesGroupDescription =
 
 STATISTIC(EmittedInsts, "Number of machine instrs printed");
 
-static cl::opt<bool> EnableRemarksSection(
-    "remarks-section",
-    cl::desc("Emit a section containing remark diagnostics metadata"),
-    cl::init(false));
-
 char AsmPrinter::ID = 0;
 
 using gcp_map_type = DenseMap<GCStrategy *, std::unique_ptr<GCMetadataPrinter>>;
@@ -1365,14 +1360,14 @@ void AsmPrinter::emitGlobalIndirectSymbol(Module &M,
   }
 }
 
-void AsmPrinter::emitRemarksSection(Module &M) {
-  RemarkStreamer *RS = M.getContext().getRemarkStreamer();
-  if (!RS)
+void AsmPrinter::emitRemarksSection(RemarkStreamer &RS) {
+  if (!RS.needsSection())
     return;
-  remarks::RemarkSerializer &RemarkSerializer = RS->getSerializer();
+
+  remarks::RemarkSerializer &RemarkSerializer = RS.getSerializer();
 
   Optional<SmallString<128>> Filename;
-  if (Optional<StringRef> FilenameRef = RS->getFilename()) {
+  if (Optional<StringRef> FilenameRef = RS.getFilename()) {
     Filename = *FilenameRef;
     sys::fs::make_absolute(*Filename);
     assert(!Filename->empty() && "The filename can't be empty.");
@@ -1427,8 +1422,8 @@ bool AsmPrinter::doFinalization(Module &M) {
   // Emit the remarks section contents.
   // FIXME: Figure out when is the safest time to emit this section. It should
   // not come after debug info.
-  if (EnableRemarksSection)
-    emitRemarksSection(M);
+  if (RemarkStreamer *RS = M.getContext().getRemarkStreamer())
+    emitRemarksSection(*RS);
 
   const TargetLoweringObjectFile &TLOF = getObjFileLowering();
 

llvm/lib/IR/RemarkStreamer.cpp

@@ -21,6 +21,13 @@
 
 using namespace llvm;
 
+static cl::opt<cl::boolOrDefault> EnableRemarksSection(
+    "remarks-section",
+    cl::desc(
+        "Emit a section containing remark diagnostics metadata. By default, "
+        "this is enabled for the following formats: yaml-strtab, bitstream."),
+    cl::init(cl::BOU_UNSET), cl::Hidden);
+
 RemarkStreamer::RemarkStreamer(
     std::unique_ptr<remarks::RemarkSerializer> RemarkSerializer,
     Optional<StringRef> FilenameIn)
@@ -104,6 +111,31 @@ void RemarkStreamer::emit(const DiagnosticInfoOptimizationBase &Diag) {
   RemarkSerializer->emit(R);
 }
 
+bool RemarkStreamer::needsSection() const {
+  if (EnableRemarksSection == cl::BOU_TRUE)
+    return true;
+
+  if (EnableRemarksSection == cl::BOU_FALSE)
+    return false;
+
+  assert(EnableRemarksSection == cl::BOU_UNSET);
+
+  // We only need a section if we're in separate mode.
+  if (RemarkSerializer->Mode != remarks::SerializerMode::Separate)
+    return false;
+
+  // Only some formats need a section:
+  // * bitstream
+  // * yaml-strtab
+  switch (RemarkSerializer->SerializerFormat) {
+  case remarks::Format::YAMLStrTab:
+  case remarks::Format::Bitstream:
+    return true;
+  default:
+    return false;
+  }
+}
+
 char RemarkSetupFileError::ID = 0;
 char RemarkSetupPatternError::ID = 0;
 char RemarkSetupFormatError::ID = 0;

llvm/test/CodeGen/X86/remarks-section.ll

@@ -2,6 +2,12 @@
 ; RUN: llc < %s -mtriple=x86_64-darwin -remarks-section -pass-remarks-output=%/t.yaml | FileCheck --check-prefix=CHECK-DARWIN -DPATH=%/t.yaml %s
 ; RUN: llc < %s -mtriple=x86_64-darwin --pass-remarks-format=yaml-strtab -remarks-section -pass-remarks-output=%/t.yaml | FileCheck --check-prefix=CHECK-DARWIN-STRTAB -DPATH=%/t.yaml %s
 
+; RUN: llc < %s -mtriple=x86_64-darwin -pass-remarks-output=%/t.yaml | FileCheck --check-prefix=CHECK-DARWIN-DEFAULT %s
+; RUN: llc < %s -mtriple=x86_64-darwin --pass-remarks-format=yaml-strtab -pass-remarks-output=%/t.yaml | FileCheck --check-prefix=CHECK-DARWIN-DEFAULT-YAML-STRTAB %s
+; RUN: llc < %s -mtriple=x86_64-darwin --pass-remarks-format=bitstream -pass-remarks-output=%/t.yaml | FileCheck --check-prefix=CHECK-DARWIN-DEFAULT-BITSTREAM %s
+; RUN: llc < %s -mtriple=x86_64-darwin --pass-remarks-format=bitstream -remarks-section=false -pass-remarks-output=%/t.yaml | FileCheck --check-prefix=CHECK-DARWIN-OVERRIDE-BITSTREAM %s
+; RUN: llc < %s -mtriple=x86_64-darwin --pass-remarks-format=yaml -remarks-section=true -pass-remarks-output=%/t.yaml | FileCheck --check-prefix=CHECK-DARWIN-OVERRIDE-YAML %s
+
 ; CHECK-LABEL: func1:
 
 ; CHECK: .section .remarks,"e",@progbits
@@ -12,6 +18,21 @@
 
 ; CHECK-DARWIN-STRTAB: .section __LLVM,__remarks,regular,debug
 ; CHECK-DARWIN-STRTAB-NEXT: .byte
+
+; By default, the format is YAML which does not need a section.
+; CHECK-DARWIN-DEFAULT-NOT: .section __LLVM,__remarks
+
+; yaml-strtab needs a section.
+; CHECK-DARWIN-DEFAULT-YAML-STRTAB: .section __LLVM,__remarks
+
+; bitstream needs a section.
+; CHECK-DARWIN-DEFAULT-BITSTREAM: .section __LLVM,__remarks
+
+; -remarks-section should force disable the section.
+; CHECK-DARWIN-OVERRIDE-BITSTREAM-NOT: .section __LLVM,__remarks
+
+; -remarks-section should also force enable the section.
+; CHECK-DARWIN-OVERRIDE-YAML: .section __LLVM,__remarks
 define void @func1() {
   ret void
 }