[PATCH] D88390: [M68K] (Patch 4/8) MC layer and object file support
Craig Topper via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 6 21:07:16 PDT 2020
craig.topper added inline comments.
================
Comment at: llvm/include/llvm/module.modulemap:83
textual header "BinaryFormat/MsgPack.def"
+ textual header "BinaryFormat/ELFRelocs/m680x0.def"
}
----------------
Alphabetize with other files
================
Comment at: llvm/lib/MC/MCExpr.cpp:228
case VK_PCREL: return "PCREL";
+ case VK_GOTPC:
+ return "GOTPC";
----------------
Put return on same line for consistency
================
Comment at: llvm/lib/MC/MCExpr.cpp:378
return StringSwitch<VariantKind>(Name.lower())
- .Case("dtprel", VK_DTPREL)
- .Case("dtpoff", VK_DTPOFF)
- .Case("got", VK_GOT)
- .Case("gotoff", VK_GOTOFF)
- .Case("gotrel", VK_GOTREL)
- .Case("pcrel", VK_PCREL)
- .Case("gotpcrel", VK_GOTPCREL)
- .Case("gottpoff", VK_GOTTPOFF)
- .Case("indntpoff", VK_INDNTPOFF)
- .Case("ntpoff", VK_NTPOFF)
- .Case("gotntpoff", VK_GOTNTPOFF)
- .Case("plt", VK_PLT)
- .Case("tlscall", VK_TLSCALL)
- .Case("tlsdesc", VK_TLSDESC)
- .Case("tlsgd", VK_TLSGD)
- .Case("tlsld", VK_TLSLD)
- .Case("tlsldm", VK_TLSLDM)
- .Case("tpoff", VK_TPOFF)
- .Case("tprel", VK_TPREL)
- .Case("tlvp", VK_TLVP)
- .Case("tlvppage", VK_TLVPPAGE)
- .Case("tlvppageoff", VK_TLVPPAGEOFF)
- .Case("page", VK_PAGE)
- .Case("pageoff", VK_PAGEOFF)
- .Case("gotpage", VK_GOTPAGE)
- .Case("gotpageoff", VK_GOTPAGEOFF)
- .Case("imgrel", VK_COFF_IMGREL32)
- .Case("secrel32", VK_SECREL)
- .Case("size", VK_SIZE)
- .Case("abs8", VK_X86_ABS8)
- .Case("l", VK_PPC_LO)
- .Case("h", VK_PPC_HI)
- .Case("ha", VK_PPC_HA)
- .Case("high", VK_PPC_HIGH)
- .Case("higha", VK_PPC_HIGHA)
- .Case("higher", VK_PPC_HIGHER)
- .Case("highera", VK_PPC_HIGHERA)
- .Case("highest", VK_PPC_HIGHEST)
- .Case("highesta", VK_PPC_HIGHESTA)
- .Case("got at l", VK_PPC_GOT_LO)
- .Case("got at h", VK_PPC_GOT_HI)
- .Case("got at ha", VK_PPC_GOT_HA)
- .Case("local", VK_PPC_LOCAL)
- .Case("tocbase", VK_PPC_TOCBASE)
- .Case("toc", VK_PPC_TOC)
- .Case("toc at l", VK_PPC_TOC_LO)
- .Case("toc at h", VK_PPC_TOC_HI)
- .Case("toc at ha", VK_PPC_TOC_HA)
- .Case("u", VK_PPC_U)
- .Case("l", VK_PPC_L)
- .Case("tls", VK_PPC_TLS)
- .Case("dtpmod", VK_PPC_DTPMOD)
- .Case("tprel at l", VK_PPC_TPREL_LO)
- .Case("tprel at h", VK_PPC_TPREL_HI)
- .Case("tprel at ha", VK_PPC_TPREL_HA)
- .Case("tprel at high", VK_PPC_TPREL_HIGH)
- .Case("tprel at higha", VK_PPC_TPREL_HIGHA)
- .Case("tprel at higher", VK_PPC_TPREL_HIGHER)
- .Case("tprel at highera", VK_PPC_TPREL_HIGHERA)
- .Case("tprel at highest", VK_PPC_TPREL_HIGHEST)
- .Case("tprel at highesta", VK_PPC_TPREL_HIGHESTA)
- .Case("dtprel at l", VK_PPC_DTPREL_LO)
- .Case("dtprel at h", VK_PPC_DTPREL_HI)
- .Case("dtprel at ha", VK_PPC_DTPREL_HA)
- .Case("dtprel at high", VK_PPC_DTPREL_HIGH)
- .Case("dtprel at higha", VK_PPC_DTPREL_HIGHA)
- .Case("dtprel at higher", VK_PPC_DTPREL_HIGHER)
- .Case("dtprel at highera", VK_PPC_DTPREL_HIGHERA)
- .Case("dtprel at highest", VK_PPC_DTPREL_HIGHEST)
- .Case("dtprel at highesta", VK_PPC_DTPREL_HIGHESTA)
- .Case("got at tprel", VK_PPC_GOT_TPREL)
- .Case("got at tprel@l", VK_PPC_GOT_TPREL_LO)
- .Case("got at tprel@h", VK_PPC_GOT_TPREL_HI)
- .Case("got at tprel@ha", VK_PPC_GOT_TPREL_HA)
- .Case("got at dtprel", VK_PPC_GOT_DTPREL)
- .Case("got at dtprel@l", VK_PPC_GOT_DTPREL_LO)
- .Case("got at dtprel@h", VK_PPC_GOT_DTPREL_HI)
- .Case("got at dtprel@ha", VK_PPC_GOT_DTPREL_HA)
- .Case("got at tlsgd", VK_PPC_GOT_TLSGD)
- .Case("got at tlsgd@l", VK_PPC_GOT_TLSGD_LO)
- .Case("got at tlsgd@h", VK_PPC_GOT_TLSGD_HI)
- .Case("got at tlsgd@ha", VK_PPC_GOT_TLSGD_HA)
- .Case("got at tlsld", VK_PPC_GOT_TLSLD)
- .Case("got at tlsld@l", VK_PPC_GOT_TLSLD_LO)
- .Case("got at tlsld@h", VK_PPC_GOT_TLSLD_HI)
- .Case("got at tlsld@ha", VK_PPC_GOT_TLSLD_HA)
- .Case("got at pcrel", VK_PPC_GOT_PCREL)
- .Case("got at tlsgd@pcrel", VK_PPC_GOT_TLSGD_PCREL)
- .Case("got at tprel@pcrel", VK_PPC_GOT_TPREL_PCREL)
- .Case("tls at pcrel", VK_PPC_TLS_PCREL)
- .Case("notoc", VK_PPC_NOTOC)
- .Case("gdgot", VK_Hexagon_GD_GOT)
- .Case("gdplt", VK_Hexagon_GD_PLT)
- .Case("iegot", VK_Hexagon_IE_GOT)
- .Case("ie", VK_Hexagon_IE)
- .Case("ldgot", VK_Hexagon_LD_GOT)
- .Case("ldplt", VK_Hexagon_LD_PLT)
- .Case("none", VK_ARM_NONE)
- .Case("got_prel", VK_ARM_GOT_PREL)
- .Case("target1", VK_ARM_TARGET1)
- .Case("target2", VK_ARM_TARGET2)
- .Case("prel31", VK_ARM_PREL31)
- .Case("sbrel", VK_ARM_SBREL)
- .Case("tlsldo", VK_ARM_TLSLDO)
- .Case("lo8", VK_AVR_LO8)
- .Case("hi8", VK_AVR_HI8)
- .Case("hlo8", VK_AVR_HLO8)
- .Case("typeindex", VK_WASM_TYPEINDEX)
- .Case("tbrel", VK_WASM_TBREL)
- .Case("mbrel", VK_WASM_MBREL)
- .Case("gotpcrel32 at lo", VK_AMDGPU_GOTPCREL32_LO)
- .Case("gotpcrel32 at hi", VK_AMDGPU_GOTPCREL32_HI)
- .Case("rel32 at lo", VK_AMDGPU_REL32_LO)
- .Case("rel32 at hi", VK_AMDGPU_REL32_HI)
- .Case("rel64", VK_AMDGPU_REL64)
- .Case("abs32 at lo", VK_AMDGPU_ABS32_LO)
- .Case("abs32 at hi", VK_AMDGPU_ABS32_HI)
- .Case("hi", VK_VE_HI32)
- .Case("lo", VK_VE_LO32)
- .Case("pc_hi", VK_VE_PC_HI32)
- .Case("pc_lo", VK_VE_PC_LO32)
- .Case("got_hi", VK_VE_GOT_HI32)
- .Case("got_lo", VK_VE_GOT_LO32)
- .Case("gotoff_hi", VK_VE_GOTOFF_HI32)
- .Case("gotoff_lo", VK_VE_GOTOFF_LO32)
- .Case("plt_hi", VK_VE_PLT_HI32)
- .Case("plt_lo", VK_VE_PLT_LO32)
- .Case("tls_gd_hi", VK_VE_TLS_GD_HI32)
- .Case("tls_gd_lo", VK_VE_TLS_GD_LO32)
- .Case("tpoff_hi", VK_VE_TPOFF_HI32)
- .Case("tpoff_lo", VK_VE_TPOFF_LO32)
- .Default(VK_Invalid);
+ .Case("dtprel", VK_DTPREL)
+ .Case("dtpoff", VK_DTPOFF)
----------------
tabs?
================
Comment at: llvm/lib/Target/M680x0/MCTargetDesc/M680x0AsmBackend.cpp:177
+ // TODO Newer CPU can use 32 bit offsets, so check for this when ready
+ if (int64_t(Value) != int64_t(int16_t(Value))) {
+ llvm_unreachable("Cannot relax the instruction, value does not fit");
----------------
Can this be !isInt<16>(Value)?
================
Comment at: llvm/lib/Target/M680x0/MCTargetDesc/M680x0AsmBackend.cpp:187
+ // displacement field contains $00 (zero offset).
+ return Value == 0 || int64_t(Value) != int64_t(int8_t(Value));
+}
----------------
isInt<8>?
================
Comment at: llvm/lib/Target/M680x0/MCTargetDesc/M680x0BaseInfo.h:251
+
+#if 0
+static inline bool isPCRelOpd(unsigned Opd) {
----------------
Can we remove commented out code or put in the patch that needs it?
================
Comment at: llvm/lib/Target/M680x0/MCTargetDesc/M680x0ELFObjectWriter.cpp:84
+ }
+ // case MCSymbolRefExpr::VK_GOT:
+ // switch (Type) {
----------------
Commented out code
================
Comment at: llvm/lib/Target/M680x0/MCTargetDesc/M680x0InstPrinter.cpp:35
+void M680x0InstPrinter::printRegName(raw_ostream &OS, unsigned RegNo) const {
+ OS << "%" << StringRef(getRegisterName(RegNo));
+}
----------------
Is the StringRef conversion needed?
================
Comment at: llvm/lib/Target/M680x0/MCTargetDesc/M680x0MCAsmInfo.cpp:29
+
+ TextAlignFillValue = 0x90;
+
----------------
Is this number correct for 68K? That looks like X86's NOP encoding.
================
Comment at: llvm/lib/Target/M680x0/MCTargetDesc/M680x0MCCodeEmitter.cpp:178
+constexpr static inline bool uintDoesFit(unsigned N, uint64_t x) {
+ return N >= 64 || x <= (UINT64_MAX >> (64 - N));
+}
----------------
Can this use isUIntN from MathExtras.h?
================
Comment at: llvm/lib/Target/M680x0/MCTargetDesc/M680x0MCCodeEmitter.cpp:182
+/// Checks if an integer fits into the given bit width. non-templated version
+constexpr static inline bool intDoesFit(unsigned N, int64_t x) {
+ return N >= 64 ||
----------------
isIntN from MathExtras?
================
Comment at: llvm/lib/Target/M680x0/MCTargetDesc/M680x0MCCodeEmitter.cpp:189
+ uint64_t &Buffer, unsigned Offset) {
+ // assert (Size && (Size == 8 || Size == 16 || Size == 32));
+ assert(Size + Offset <= 64 && "Value does not fit");
----------------
Commented out code?
================
Comment at: llvm/lib/Target/M680x0/MCTargetDesc/M680x0MCCodeEmitter.cpp:344
+ const MCInstrDesc &Desc = MCII.get(Opcode);
+ // uint64_t TSFlags = Desc.TSFlags;
+
----------------
Commented out code
================
Comment at: llvm/lib/Target/M680x0/MCTargetDesc/M680x0MCCodeEmitter.cpp:407
+
+ // while (Offset >= 8) {
+ // OS.write((char)(Buffer & 0xFF));
----------------
Commented out code
================
Comment at: llvm/lib/Target/M680x0/MCTargetDesc/M680x0MCTargetDesc.cpp:65
+ if (!ArchFS.empty()) {
+ ArchFS = (ArchFS + "," + FS).getSingleStringRef();
+ } else {
----------------
I'm not sure the result of a concatenation can be a "SingleStringRef"
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88390/new/
https://reviews.llvm.org/D88390
More information about the llvm-commits
mailing list