Adding skeleton for unit testing Hexagon Code Emission

[colinl]

Summary

Adding and modifying CMakeLists.txt files to run unit tests under unittests/Target/* if the directory exists.
Adding basic unit test to check that code emitter object can be retrieved.

[mcrosier]

lib/Target/Hexagon/MCTargetDesc/HexagonMCCodeEmitter.cpp

35

Please add a period.

38

No need for curly brackets, if the body is a single statement.

39

Where are these magic numbers coming from?

74

Reduce indent and remove curly brackets.

if (MO.isReg())

return MCT.getRegisterInfo()->getEncodingValue(MO.getReg());

if (MO.isImm())

return static_cast<unsigned>(MO.getImm());

unittests/Target/CMakeLists.txt

6 ↗

(On Diff #14167)

Please add newline.

unittests/Target/Hexagon/CMakeLists.txt

11 ↗

(On Diff #14167)

Please add newline.

unittests/Target/Hexagon/HexagonMCCodeEmitterTest.cpp

54 ↗

(On Diff #14167)

Please add newline.

[colinl]

Applying formatting updates.
Adding enumeration to explain bitfield and constant for bitfield offsets.

[mcrosier]

Once the comments are address, I believe this is in good enough shape. Please upload a revised patch and I'll approve.

lib/Target/Hexagon/MCTargetDesc/HexagonMCCodeEmitter.cpp

11

IIRC, I believe this should be added after the #includes.

42

How about something like this?

ParseField Field = HMI.isPacketEnd() ? ParseField::end : ParseField::last0;
return static_cast<uint32_t>(Field) << ParseFieldOffset;

unittests/CMakeLists.txt

26 ↗

(On Diff #14185)

Would this be better in

unittests/MC/Target

or

unittests/MC/Hexagon (I prefer)

?

The latter suggestion is more inline with how test/MC/Hexagon and test/CodeGen/Hexagon are laid out.

unittests/Target/Hexagon/HexagonMCCodeEmitterTest.cpp

23 ↗

(On Diff #14185)

assert(Target && "Expected to find a target.");

24 ↗

(On Diff #14185)

Please use the
assert(cond && "msg");
style.

Many more instances below.

[mcrosier]

Sorry for the multiple reviews.. Please address these as well.

lib/Target/Hexagon/MCTargetDesc/HexagonMCCodeEmitter.cpp

65

How about just:

uint64_t Binary = getBinaryCodeForInstr(HMB, Fixups, STI) | getPacketBits(HMB);

68

assert(cond && "msg");

[mcrosier]

LGTM.

[sidneym]

Looks good and I will submit this for you today. Chris Lattner can grant you check-in permission.

[emaste]

lib/Target/Hexagon/CMakeLists.txt

5

Needs to be added to Makefile too?

[colinl]

Patch applied.

lib/Target/Hexagon/CMakeLists.txt

 
 tablegen(LLVM HexagonGenRegisterInfo.inc -gen-register-info)
 tablegen(LLVM HexagonGenInstrInfo.inc -gen-instr-info)
+tablegen(LLVM HexagonGenMCCodeEmitter.inc -gen-emitter)
 tablegen(LLVM HexagonGenAsmWriter.inc -gen-asm-writer)
 tablegen(LLVM HexagonGenDAGISel.inc -gen-dag-isel)
 tablegen(LLVM HexagonGenCallingConv.inc -gen-callingconv)

lib/Target/Hexagon/MCTargetDesc/CMakeLists.txt

 add_llvm_library(LLVMHexagonDesc
   HexagonMCAsmInfo.cpp
+  HexagonMCCodeEmitter.cpp
   HexagonMCInst.cpp
   HexagonMCTargetDesc.cpp
   )
+
+add_dependencies(LLVMHexagonDesc HexagonCommonTableGen)
 No newline at end of file

lib/Target/Hexagon/MCTargetDesc/HexagonMCCodeEmitter.h

+//===-- HexagonMCCodeEmitter.h - Hexagon Target Descriptions ----*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// \brief Definition for classes that emit Hexagon machine code from MCInsts
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef HEXAGONMCCODEEMITTER_H
+#define HEXAGONMCCODEEMITTER_H
+
+#include "llvm/MC/MCCodeEmitter.h"
+#include "llvm/MC/MCExpr.h"
+#include "llvm/MC/MCInst.h"
+#include "llvm/MC/MCInstrInfo.h"
+#include "llvm/MC/MCRegisterInfo.h"
+#include "llvm/MC/MCSubtargetInfo.h"
+#include "llvm/Support/raw_ostream.h"
+
+namespace llvm {
+
+class HexagonMCCodeEmitter : public MCCodeEmitter {
+  MCSubtargetInfo const &MST;
+  MCContext &MCT;
+
+public:
+  HexagonMCCodeEmitter(MCInstrInfo const &aMII, MCSubtargetInfo const &aMST,
+                       MCContext &aMCT);
+
+  MCSubtargetInfo const &getSubtargetInfo() const;
+
+  void EncodeInstruction(MCInst const &MI, raw_ostream &OS,
+                         SmallVectorImpl<MCFixup> &Fixups,
+                         MCSubtargetInfo const &STI) const override;
+
+  // getBinaryCodeForInstr - TableGen'erated function for getting the
+  // binary encoding for an instruction.
+  uint64_t getBinaryCodeForInstr(MCInst const &MI,
+                                 SmallVectorImpl<MCFixup> &Fixups,
+                                 MCSubtargetInfo const &STI) const;
+
+  /// \brief Return binary encoding of operand.
+  unsigned getMachineOpValue(MCInst const &MI, MCOperand const &MO,
+                             SmallVectorImpl<MCFixup> &Fixups,
+                             MCSubtargetInfo const &STI) const;
+
+private:
+  HexagonMCCodeEmitter(HexagonMCCodeEmitter const &) = delete;
+  void operator=(HexagonMCCodeEmitter const &) = delete;
+}; // class HexagonMCCodeEmitter
+
+} // namespace llvm
+
+#endif /* HEXAGONMCCODEEMITTER_H */

lib/Target/Hexagon/MCTargetDesc/HexagonMCCodeEmitter.cpp

+//===-- HexagonMCCodeEmitter.cpp - Hexagon Target Descriptions ------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "Hexagon.h"
+#include "MCTargetDesc/HexagonBaseInfo.h"
+#include "MCTargetDesc/HexagonMCCodeEmitter.h"
+#include "MCTargetDesc/HexagonMCTargetDesc.h"
+#include "MCTargetDesc/HexagonMCInst.h"
+#include "llvm/ADT/Statistic.h"
+#include "llvm/MC/MCCodeEmitter.h"
+#include "llvm/MC/MCContext.h"
+#include "llvm/MC/MCExpr.h"
+#include "llvm/MC/MCInst.h"
+#include "llvm/MC/MCInstrInfo.h"
+#include "llvm/MC/MCRegisterInfo.h"
+#include "llvm/MC/MCSubtargetInfo.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/raw_ostream.h"
+
+#define DEBUG_TYPE "mccodeemitter"
+
+using namespace llvm;
+using namespace Hexagon;
+
+STATISTIC(MCNumEmitted, "Number of MC instructions emitted");
+
+namespace {
+/// \brief 10.6 Instruction Packets
+/// \brief Possible values for instruction packet parse field.
+enum class ParseField { duplex = 0x0, last0 = 0x1, last1 = 0x2, end = 0x3 };
+/// \brief Returns the packet bits based on instruction position.
+uint32_t getPacketBits(HexagonMCInst const &HMI) {
+  unsigned const ParseFieldOffset = 14;
+  ParseField Field = HMI.isPacketEnd() ? ParseField::end : ParseField::last0;
+  return static_cast <uint32_t> (Field) << ParseFieldOffset;
+}
+void emitLittleEndian(uint64_t Binary, raw_ostream &OS) {
+  OS << static_cast<uint8_t>((Binary >> 0x00) & 0xff);
+  OS << static_cast<uint8_t>((Binary >> 0x08) & 0xff);
+  OS << static_cast<uint8_t>((Binary >> 0x10) & 0xff);
+  OS << static_cast<uint8_t>((Binary >> 0x18) & 0xff);
+}
+}
+
+HexagonMCCodeEmitter::HexagonMCCodeEmitter(MCInstrInfo const &aMII,
+                                           MCSubtargetInfo const &aMST,
+                                           MCContext &aMCT)
+    : MST(aMST), MCT(aMCT) {}
+
+void HexagonMCCodeEmitter::EncodeInstruction(MCInst const &MI, raw_ostream &OS,
+                                             SmallVectorImpl<MCFixup> &Fixups,
+                                             MCSubtargetInfo const &STI) const {
+  HexagonMCInst const &HMB = static_cast<HexagonMCInst const &>(MI);
+  uint64_t Binary = getBinaryCodeForInstr(HMB, Fixups, STI) | getPacketBits(HMB);
+  assert(HMB.getDesc().getSize() == 4 && "All instructions should be 32bit");
+  emitLittleEndian(Binary, OS);
+  ++MCNumEmitted;
+}
+
+unsigned
+HexagonMCCodeEmitter::getMachineOpValue(MCInst const &MI, MCOperand const &MO,
+                                        SmallVectorImpl<MCFixup> &Fixups,
+                                        MCSubtargetInfo const &STI) const {
+  if (MO.isReg())
+    return MCT.getRegisterInfo()->getEncodingValue(MO.getReg());
+  if (MO.isImm())
+    return static_cast<unsigned>(MO.getImm());
+  llvm_unreachable("Only Immediates and Registers implemented right now");
+}
+
+MCSubtargetInfo const &HexagonMCCodeEmitter::getSubtargetInfo() const {
+  return MST;
+}
+
+MCCodeEmitter *llvm::createHexagonMCCodeEmitter(MCInstrInfo const &MII,
+                                                MCRegisterInfo const &MRI,
+                                                MCSubtargetInfo const &MST,
+                                                MCContext &MCT) {
+  return new HexagonMCCodeEmitter(MII, MST, MCT);
+}
+
+#include "HexagonGenMCCodeEmitter.inc"

lib/Target/Hexagon/MCTargetDesc/HexagonMCTargetDesc.h

 #define LLVM_LIB_TARGET_HEXAGON_MCTARGETDESC_HEXAGONMCTARGETDESC_H
 
 namespace llvm {
+class MCCodeEmitter;
+class MCContext;
+class MCInstrInfo;
+class MCRegisterInfo;
 class MCSubtargetInfo;
 class Target;
 
 extern Target TheHexagonTarget;
 
+MCCodeEmitter *createHexagonMCCodeEmitter(const MCInstrInfo &MCII,
+                                          const MCRegisterInfo &MRI,
+                                          const MCSubtargetInfo &MST,
+                                          MCContext &MCT);
+
 } // End llvm namespace
 
 // Define symbolic names for Hexagon registers.  This defines a mapping from

lib/Target/Hexagon/MCTargetDesc/HexagonMCTargetDesc.cpp

   return X;
 }
 
-static MCSubtargetInfo *createHexagonMCSubtargetInfo(StringRef TT,
-                                                     StringRef CPU,
-                                                     StringRef FS) {
+static MCSubtargetInfo *
+createHexagonMCSubtargetInfo(StringRef TT, StringRef CPU, StringRef FS) {
   MCSubtargetInfo *X = new MCSubtargetInfo();
   InitHexagonMCSubtargetInfo(X, TT, CPU, FS);
   return X;

   MCAsmInfo *MAI = new HexagonMCAsmInfo(TT);
 
   // VirtualFP = (R30 + #0).
-  MCCFIInstruction Inst = MCCFIInstruction::createDefCfa(
-      nullptr, Hexagon::R30, 0);
+  MCCFIInstruction Inst =
+      MCCFIInstruction::createDefCfa(nullptr, Hexagon::R30, 0);
   MAI->addInitialFrameState(Inst);
 
   return MAI;

 }
 
 static MCCodeGenInfo *createHexagonMCCodeGenInfo(StringRef TT, Reloc::Model RM,
                                                  CodeModel::Model CM,
                                                  CodeGenOpt::Level OL) {
   MCCodeGenInfo *X = new MCCodeGenInfo();
   // For the time being, use static relocations, since there's really no
   // support for PIC yet.

                                         createHexagonMCCodeGenInfo);
 
   // Register the MC instruction info.
-  TargetRegistry::RegisterMCInstrInfo(TheHexagonTarget, createHexagonMCInstrInfo);
+  TargetRegistry::RegisterMCInstrInfo(TheHexagonTarget,
+                                      createHexagonMCInstrInfo);
 
   // Register the MC register info.
   TargetRegistry::RegisterMCRegInfo(TheHexagonTarget,

   // Register the MC subtarget info.
   TargetRegistry::RegisterMCSubtargetInfo(TheHexagonTarget,
                                           createHexagonMCSubtargetInfo);
+
+  // Register the MC Code Emitter
+  TargetRegistry::RegisterMCCodeEmitter(TheHexagonTarget,
+                                        createHexagonMCCodeEmitter);
 }

unittests/MC/CMakeLists.txt

   StringTableBuilderTest.cpp
   YAMLTest.cpp
   )
+  
+foreach(t ${LLVM_TARGETS_TO_BUILD})
+  if (IS_DIRECTORY "${CMAKE_CURRENT_LIST_DIR}/${t}")
+    add_subdirectory(${t})
+  endif (IS_DIRECTORY "${CMAKE_CURRENT_LIST_DIR}/${t}")
+endforeach()

unittests/MC/Hexagon/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS
+  HexagonCodeGen
+  )
+
+include_directories (${LLVM_MAIN_SRC_DIR}/lib/Target/Hexagon)
+include_directories (${LLVM_BINARY_DIR}/lib/Target/Hexagon)
+
+add_llvm_unittest(HexagonTests
+  HexagonMCCodeEmitterTest.cpp
+  )

unittests/MC/Hexagon/HexagonMCCodeEmitterTest.cpp

+#include "gtest/gtest.h"
+
+#include <memory>
+
+#include "llvm/MC/MCCodeEmitter.h"
+#include "llvm/MC/MCContext.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/TargetRegistry.h"
+#include "llvm/Support/TargetSelect.h"
+
+#include "MCTargetDesc/HexagonMCInst.h"
+#include "MCTargetDesc/HexagonMCTargetDesc.h"
+
+namespace {
+class TestEmitter {
+public:
+  TestEmitter() : Triple("hexagon-unknown-elf") {
+    LLVMInitializeHexagonTargetInfo();
+    LLVMInitializeHexagonTarget();
+    LLVMInitializeHexagonTargetMC();
+    std::string error;
+    Target = llvm::TargetRegistry::lookupTarget("hexagon", error);
+    assert(Target != nullptr && "Expected to find target");
+    assert(error.empty() && "Error should be empty if we have a target");
+    RegisterInfo = Target->createMCRegInfo(Triple);
+    assert(RegisterInfo != nullptr && "Expecting to find register info");
+    AsmInfo = Target->createMCAsmInfo(*RegisterInfo, Triple);
+    assert(AsmInfo != nullptr && "Expecting to find asm info");
+    Context = new llvm::MCContext(AsmInfo, RegisterInfo, nullptr);
+    assert(Context != nullptr && "Expecting to create a context");
+    Subtarget = Target->createMCSubtargetInfo(Triple, "hexagonv4", "");
+    assert(Subtarget != nullptr && "Expecting to find a subtarget");
+    InstrInfo = Target->createMCInstrInfo();
+    assert(InstrInfo != nullptr && "Expecting to find instr info");
+    Emitter = Target->createMCCodeEmitter(*InstrInfo, *RegisterInfo, *Subtarget,
+                                          *Context);
+    assert(Emitter != nullptr);
+  }
+  std::string Triple;
+  llvm::Target const *Target;
+  llvm::MCRegisterInfo *RegisterInfo;
+  llvm::MCAsmInfo *AsmInfo;
+  llvm::MCContext *Context;
+  llvm::MCSubtargetInfo *Subtarget;
+  llvm::MCInstrInfo *InstrInfo;
+  llvm::MCCodeEmitter *Emitter;
+};
+TestEmitter Emitter;
+}
+
+TEST(HexagonMCCodeEmitter, emitter_creation) {
+  ASSERT_NE(nullptr, Emitter.Emitter);
+}