[RISCV] Collect Statistics on Compressed Instructions

[lenary]

Summary

It is useful to keep statistics on how many instructions we have
compressed, so we can see if future changes are increasing or decreasing this
number.

[luismarques]

LGTM. My only concern was if it made sense to use the same statistic to count in both places, and if we could end up double counting the instructions emitted (now, or in a future LLVM version). After a quick look I didn't really see other targets using the same approach, but I also can't think of a way where this ends up actually being problematic.

(Nipick: maybe put a comment in the statistic reminding people that the linker may further compress some of the instructions we emitted uncompressed?)

[xbolva00]

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

1618–1621

under if (Res) too ?

[lenary]

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

1618–1621

This was covered in rGa0f43b0043581b37b10d105a85f0653704d3657b - I just need to rebase this patch.

[lenary]

Rebase

[lenary]

In D67495#1800935, @luismarques wrote:

LGTM. My only concern was if it made sense to use the same statistic to count in both places, and if we could end up double counting the instructions emitted (now, or in a future LLVM version). After a quick look I didn't really see other targets using the same approach, but I also can't think of a way where this ends up actually being problematic.

My understanding is that instructions only go through one of these functions. RISCVAsmParser.cpp is used by the assembler only, and the RISCVAsmPrinter.cpp is only used by LLVM CodeGen. This should mean that instructions are not double-counted (today). Yes I'm not sure why we have both, I think @asb did this to ensure better layering.

Note, the statistics are not the same. Statistics are namespaced by DEBUG_TYPE, which is why I had to define it in RISCVAsmParser.cpp.

(Nipick: maybe put a comment in the statistic reminding people that the linker may further compress some of the instructions we emitted uncompressed?)

The point in these statistics is to track how good LLVM is at compressing instructions (which may change depending on optimisation heuristics we implement eventually). I don't see this as being used by end users much, if at all.

[asb]

In D67495#1811715, @lenary wrote:

In D67495#1800935, @luismarques wrote:

LGTM. My only concern was if it made sense to use the same statistic to count in both places, and if we could end up double counting the instructions emitted (now, or in a future LLVM version). After a quick look I didn't really see other targets using the same approach, but I also can't think of a way where this ends up actually being problematic.

My understanding is that instructions only go through one of these functions. RISCVAsmParser.cpp is used by the assembler only, and the RISCVAsmPrinter.cpp is only used by LLVM CodeGen. This should mean that instructions are not double-counted (today). Yes I'm not sure why we have both, I think @asb did this to ensure better layering.

Both are necessary for completeness, and are independent paths for MCInst generation. We did go back and forth a fair bit on exactly where the compression hook should be called and that seemed the most sensible option - I forget the full analysis, but there wasn't a single place we could stick it while remaining localised to the RISC-V backend.

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

 #include "MCTargetDesc/RISCVMCExpr.h"
 #include "MCTargetDesc/RISCVMCTargetDesc.h"
 #include "MCTargetDesc/RISCVTargetStreamer.h"
 #include "TargetInfo/RISCVTargetInfo.h"
 #include "Utils/RISCVBaseInfo.h"
 #include "Utils/RISCVMatInt.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/CodeGen/Register.h"
 #include "llvm/MC/MCAssembler.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCInstBuilder.h"
 #include "llvm/MC/MCObjectFileInfo.h"
 #include "llvm/MC/MCParser/MCAsmLexer.h"
 #include "llvm/MC/MCParser/MCParsedAsmOperand.h"
 #include "llvm/MC/MCParser/MCTargetAsmParser.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/TargetRegistry.h"
 
 #include <limits>
 
 using namespace llvm;
 
+#define DEBUG_TYPE "riscv-asm-parser"
+
 // Include the auto-generated portion of the compress emitter.
 #define GEN_COMPRESS_INSTR
 #include "RISCVGenCompressInstEmitter.inc"
 
+STATISTIC(RISCVNumInstrsCompressed,
+          "Number of RISC-V Compressed instructions emitted");
+
 namespace {
 struct RISCVOperand;
 
 class RISCVAsmParser : public MCTargetAsmParser {
   SmallVector<FeatureBitset, 4> FeatureBitStack;
 
   SMLoc getLoc() const { return getParser().getTok().getLoc(); }
   bool isRV64() const { return getSTI().hasFeature(RISCV::Feature64Bit); }
Warning(Parser.getTok().getLoc(),
           "unknown option, expected 'push', 'pop', 'rvc', 'norvc', 'relax' or "
           "'norelax'");
   Parser.eatToEndOfStatement();
   return false;
 }
 
 void RISCVAsmParser::emitToStreamer(MCStreamer &S, const MCInst &Inst) {
   MCInst CInst;
   bool Res = compressInst(CInst, Inst, getSTI(), S.getContext());
+  if (Res)
+    ++RISCVNumInstrsCompressed;
   S.EmitInstruction((Res ? CInst : Inst), getSTI());
 }
 
 void RISCVAsmParser::emitLoadImm(Register DestReg, int64_t Value,
                                  MCStreamer &Out) {
   RISCVMatInt::InstSeq Seq;
   RISCVMatInt::generateInstSeq(Value, isRV64(), Seq);
 
   Register SrcReg = RISCV::X0;

llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "RISCV.h"
 #include "MCTargetDesc/RISCVInstPrinter.h"
 #include "MCTargetDesc/RISCVMCExpr.h"
 #include "RISCVTargetMachine.h"
 #include "TargetInfo/RISCVTargetInfo.h"
+#include "llvm/ADT/Statistic.h"
 #include "llvm/CodeGen/AsmPrinter.h"
 #include "llvm/CodeGen/MachineConstantPool.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineInstr.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "asm-printer"
 
+STATISTIC(RISCVNumInstrsCompressed,
+          "Number of RISC-V Compressed instructions emitted");
+
 namespace {
 class RISCVAsmPrinter : public AsmPrinter {
 public:
   explicit RISCVAsmPrinter(TargetMachine &TM,
                            std::unique_ptr<MCStreamer> Streamer)
       : AsmPrinter(TM, std::move(Streamer)) {}
 
   StringRef getPassName() const override { return "RISCV Assembly Printer"; }

 }
 
 #define GEN_COMPRESS_INSTR
 #include "RISCVGenCompressInstEmitter.inc"
 void RISCVAsmPrinter::EmitToStreamer(MCStreamer &S, const MCInst &Inst) {
   MCInst CInst;
   bool Res = compressInst(CInst, Inst, *TM.getMCSubtargetInfo(),
                           OutStreamer->getContext());
+  if (Res)
+    ++RISCVNumInstrsCompressed;
   AsmPrinter::EmitToStreamer(*OutStreamer, Res ? CInst : Inst);
 }
 
 // Simple pseudo-instructions have their lowering (with expansion to real
 // instructions) auto-generated.
 #include "RISCVGenMCPseudoLowering.inc"
 
 void RISCVAsmPrinter::EmitInstruction(const MachineInstr *MI) {