[PATCH] D41658: [RISCV] Encode RISCV specific ELF e_flags to RISCV Binary by RISCVTargetStreamer

[Ana Pazos via Phabricator via llvm-commits]

apazos added inline comments.


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp:73
+createRISCVObjectTargetStreamer(MCStreamer &S, const MCSubtargetInfo &STI) {
+  return new RISCVTargetELFStreamer(S, STI);
+}
----------------
Should'nt we check for ELF format, that is the only one supported now, but may change later? 

const Triple &TT = STI.getTargetTriple();
  if (TT.isOSBinFormatELF())
    return new RISCVTargetELFStreamer(S);
  return nullptr;


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp:85
     TargetRegistry::RegisterMCSubtargetInfo(*T, createRISCVMCSubtargetInfo);
+    TargetRegistry::RegisterObjectTargetStreamer(
+        *T, createRISCVObjectTargetStreamer);
----------------
Reading MCStreamer.h I understand when we define a target streamer we need to define two streamers, ELF and Asm. Is it how you interpreted?

/// Target specific streamer interface. This is used so that targets can
/// implement support for target specific assembly directives.
///
/// If target foo wants to use this, it should implement 3 classes:
/// * FooTargetStreamer : public MCTargetStreamer
/// * FooTargetAsmStreamer : public FooTargetStreamer
/// * FooTargetELFStreamer : public FooTargetStreamer



================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp:41
+MCELFStreamer &RISCVTargetELFStreamer::getStreamer() {
+  return static_cast<MCELFStreamer &>(Streamer);
+}
----------------
Shouldn't it be return static_cast<RISCVELFStreamer &>(Streamer);


Repository:
  rL LLVM

https://reviews.llvm.org/D41658