[RFC][mips][llvm-exegesis] Fix missing TargetStreamer in the Streamer for reading snippets

[mstojanovic]

Summary

To extract instructions from custom code snippets, llvm-exegesis creates a minimal parser in readSnippets(). The parser doesn't initialize the TargetStreamer because it's not used on X86 (and most other architectures) but the MipsAsmParser assumes that it is initialized which creates a problem when extending the implementation.

One solution could be to add the initialization of the TargetStreamer, but the functions that can be used for that usually create arch specific streamers like MipsTargetELFStreamer which (again, unlike most other architectures) needs an Assembler which isn't initialized. Since the BenchmarkStreamer only inherits the MCStreamer it's unclear what type of reorganization would be needed to accommodate additional information like the Assembler.

Another solution, submitted here, is to check whether TargetStreamer returns a null reference (which AFAIK isn't allowed by definition but still happens) from the Mips side and skip accessing it. This might miss edge cases or future uses where the TargetStreamer is erroneously missing but it works for this case. The TargetStreamer is usally used in cases where some additional instructions need to be emitted, but since the tool is only reading the snippets most of the code should be skipped, so there shouldn't be problems with not initilizing variables like TOut. This is just a basic patch which enables running exegesis with an empty file: llvm-exegesis -mode latency -snippets-file=/dev/null or a file containing simple instructions. To be able to run this for DIV for example, TOut in expandDivRem() would have to adapted in a similar way, and so on for other instructions. A possible improvement of this solution could be to use dyn_cast to determine the origin of the Streamer instead of assuming that it's from exegesis if the TargetStreamer isn't initialized. This would requires dyn_cast to be implemented for streamers which it currently isn't.

These are some ideas that don't constitute a perfect solution so I hope you can help me get a clearer insight into how to a better one might look like.

[atanasyan]

AsmParsers in other architectures have references and use a TargetStreamer too. What's a difference in MIPS? Does MIPS need a TargetStreamer in places where other architectures do not use it?

[mstojanovic]

Yes, other targets do have it but use it quite sparingly. X86 almost never uses it but there are some edge cases when parsing certain directives. For example, this line will trigger an assert fail because of an uninitialized TargetStreamer on X86:

echo ".cv_fpo_proc foo 4" | llvm-exegesis -mode latency -snippets-file=-

MIPS on the other hand uses TargetStreamer right at the start of the MipsAsmParser constructor and throughout the code used for parsing. The design of this MIPS code is quite different from other architectures (always relying on TargetStreamer or Assembler). I'm not sure if this was out of necessity or if it just evolved in to different path. Either way, the current code doesn't facilitate simple instruction extraction without all the necessary elements being in place.

[mstojanovic]

@gchatelet @courbet How do you think the example I mentioned in the previous comment should be handeled on X86? This might inform us what (if anything) should be changed for MIPS.

[gchatelet]

In D71137#1781876, @mstojanovic wrote:

@gchatelet @courbet How do you think the example I mentioned in the previous comment should be handeled on X86? This might inform us what (if anything) should be changed for MIPS.

Thx for taking the time to explain the offered solution in great details.
I need to see how we can adapt the code to get the Streamer created in the first place because it looks like it should be set up properly.
I've been busy the last few days but I'll have a look shortly (same for the other patch you sent my way).

My apologies for the delay.

[gchatelet]

I've created D71468 which should fix the issue for all backend at once and prevent hacking in MipsAsmParser.cpp

llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp

const FeatureBitset MipsAssemblerOptions::AllArchRelatedMask = {
     Mips::FeatureCnMipsP, Mips::FeatureFP64Bit, Mips::FeatureGP64Bit,
     Mips::FeatureNaN2008
 };
 
 namespace {
 
 class MipsAsmParser : public MCTargetAsmParser {
   MipsTargetStreamer &getTargetStreamer() {
+    assert(getParser().getStreamer().getTargetStreamer() &&
+           "do not have a target streamer");
     MCTargetStreamer &TS = *getParser().getStreamer().getTargetStreamer();
     return static_cast<MipsTargetStreamer &>(TS);
   }
 
+  // The llvm-exegesis Streamer used for parsing code snippets doesn't have the
+  // TargetStreamer initialized.
+  bool isExegesisStreamer() {
+    return getParser().getStreamer().getTargetStreamer();
+  }
+
   MipsABIInfo ABI;
   SmallVector<std::unique_ptr<MipsAssemblerOptions>, 2> AssemblerOptions;
   MCSymbol *CurrentFn; // Pointer to the function being parsed. It may be a
                        // nullptr, which indicates that no function is currently
                        // selected. This usually happens after an '.end func'
                        // directive.
   bool IsLittleEndian;
   bool IsPicEnabled;
MipsAsmParser(const MCSubtargetInfo &sti, MCAsmParser &parser,
     // Remember the initial assembler options. The user can not modify these.
     AssemblerOptions.push_back(
         std::make_unique<MipsAssemblerOptions>(getSTI().getFeatureBits()));
 
     // Create an assembler options environment for the user to modify.
     AssemblerOptions.push_back(
         std::make_unique<MipsAssemblerOptions>(getSTI().getFeatureBits()));
 
+    if (isExegesisStreamer())
       getTargetStreamer().updateABIInfo(*this);
 
     if (!isABI_O32() && !useOddSPReg() != 0)
       report_fatal_error("-mno-odd-spreg requires the O32 ABI");
 
     CurrentFn = nullptr;
 
     IsPicEnabled = getContext().getObjectFileInfo()->isPositionIndependent();
 
static bool needsExpandMemInst(MCInst &Inst) {
   }
 
   return false;
 }
 
 bool MipsAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
                                        MCStreamer &Out,
                                        const MCSubtargetInfo *STI) {
-  MipsTargetStreamer &TOut = getTargetStreamer();
+  MipsTargetStreamer *TOut;
+  if (isExegesisStreamer())
+    TOut = &getTargetStreamer();
   const unsigned Opcode = Inst.getOpcode();
   const MCInstrDesc &MCID = getInstDesc(Opcode);
   bool ExpandedJalSym = false;
 
   Inst.setLoc(IDLoc);
 
   if (MCID.isBranch() || MCID.isCall()) {
     MCOperand Offset;
if (isJalrRelocAvailable(JalExpr)) {
       // .reloc tmplabel, R_{MICRO}MIPS_JALR, symbol
       // tmplabel:
       MCSymbol *TmpLabel = getContext().createTempSymbol();
       const MCExpr *TmpExpr = MCSymbolRefExpr::create(TmpLabel, getContext());
       const MCExpr *RelocJalrExpr =
           MCSymbolRefExpr::create(JalSym, MCSymbolRefExpr::VK_None,
                                   getContext(), IDLoc);
 
-      TOut.getStreamer().EmitRelocDirective(*TmpExpr,
+      TOut->getStreamer().EmitRelocDirective(*TmpExpr,
           inMicroMipsMode() ? "R_MICROMIPS_JALR" : "R_MIPS_JALR",
           RelocJalrExpr, IDLoc, *STI);
-      TOut.getStreamer().EmitLabel(TmpLabel);
+      TOut->getStreamer().EmitLabel(TmpLabel);
     }
 
     Inst = JalrInst;
     ExpandedJalSym = true;
   }
 
   if (MCID.mayLoad() || MCID.mayStore()) {
     // Check the offset of memory operand, if it is a symbol
if (MCID.mayLoad() && Opcode != Mips::LWP_MM) {
             MCOperand &DstReg = Inst.getOperand(0);
             MCOperand &BaseReg = Inst.getOperand(1);
             if (isInt<9>(MemOffset) && (MemOffset % 4 == 0) &&
                 getContext().getRegisterInfo()->getRegClass(
                   Mips::GPRMM16RegClassID).contains(DstReg.getReg()) &&
                 (BaseReg.getReg() == Mips::GP ||
                 BaseReg.getReg() == Mips::GP_64)) {
 
-              TOut.emitRRI(Mips::LWGP_MM, DstReg.getReg(), Mips::GP, MemOffset,
+              TOut->emitRRI(Mips::LWGP_MM, DstReg.getReg(), Mips::GP, MemOffset,
                            IDLoc, STI);
               return false;
             }
           }
         }
       } // for
     }   // if load
 
switch (Opcode) {
         break;
       }
     }
   }
 
   bool FillDelaySlot =
       MCID.hasDelaySlot() && AssemblerOptions.back()->isReorder();
   if (FillDelaySlot)
-    TOut.emitDirectiveSetNoReorder();
+    TOut->emitDirectiveSetNoReorder();
 
   MacroExpanderResultTy ExpandResult =
       tryExpandInstruction(Inst, IDLoc, Out, STI);
   switch (ExpandResult) {
   case MER_NotAMacro:
     Out.EmitInstruction(Inst, *STI);
     break;
   case MER_Success:
     break;
   case MER_Fail:
     return true;
   }
 
   // We know we emitted an instruction on the MER_NotAMacro or MER_Success path.
   // If we're in microMIPS mode then we must also set EF_MIPS_MICROMIPS.
   if (inMicroMipsMode()) {
-    TOut.setUsesMicroMips();
-    TOut.updateABIInfo(*this);
+    TOut->setUsesMicroMips();
+    TOut->updateABIInfo(*this);
   }
 
   // If this instruction has a delay slot and .set reorder is active,
   // emit a NOP after it.
   if (FillDelaySlot) {
-    TOut.emitEmptyDelaySlot(hasShortDelaySlot(Inst), IDLoc, STI);
-    TOut.emitDirectiveSetReorder();
+    TOut->emitEmptyDelaySlot(hasShortDelaySlot(Inst), IDLoc, STI);
+    TOut->emitDirectiveSetReorder();
   }
 
   if ((Opcode == Mips::JalOneReg || Opcode == Mips::JalTwoReg ||
        ExpandedJalSym) &&
       isPicAndNotNxxAbi()) {
     if (IsCpRestoreSet) {
       // We need a NOP between the JALR and the LW:
       // If .set reorder has been used, we've already emitted a NOP.
       // If .set noreorder has been used, we need to emit a NOP at this point.
       if (!AssemblerOptions.back()->isReorder())
-        TOut.emitEmptyDelaySlot(hasShortDelaySlot(Inst), IDLoc,
+        TOut->emitEmptyDelaySlot(hasShortDelaySlot(Inst), IDLoc,
                                 STI);
 
       // Load the $gp from the stack.
-      TOut.emitGPRestore(CpRestoreOffset, IDLoc, STI);
+      TOut->emitGPRestore(CpRestoreOffset, IDLoc, STI);
     } else
       Warning(IDLoc, "no .cprestore used in PIC mode");
   }
 
   return false;
 }
 
 MipsAsmParser::MacroExpanderResultTy
static std::string MipsMnemonicSpellCheck(StringRef S, const FeatureBitset &FBS,
                                           unsigned VariantID = 0);
 
 bool MipsAsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
                                      SMLoc NameLoc, OperandVector &Operands) {
   MCAsmParser &Parser = getParser();
   LLVM_DEBUG(dbgs() << "ParseInstruction\n");
 
   // We have reached first instruction, module directive are now forbidden.
+  if (isExegesisStreamer())
     getTargetStreamer().forbidModuleDirective();
 
   // Check if we have valid mnemonic
   if (!mnemonicIsValid(Name, 0)) {
     FeatureBitset FBS = ComputeAvailableFeatures(getSTI().getFeatureBits());
     std::string Suggestion = MipsMnemonicSpellCheck(Name, FBS);
     return Error(NameLoc, "unknown instruction" + Suggestion);
   }
   // First operand in MCInst is instruction mnemonic.