From 0b58446fa38729846ea9c48bcbe96b001f3857df Mon Sep 17 00:00:00 2001 From: Matt Arsenault Date: Wed, 22 May 2019 16:28:41 +0000 Subject: [PATCH] MC: Allow getMaxInstLength to depend on the subtarget Keep it optional in cases this is ever needed in some global context. Currently it's only used for getting an upper bound inline asm code size. For AMDGPU, gfx10 increases the maximum instruction size to 20-bytes. This avoids penalizing older subtargets when estimating code size, and making some annoying branch relaxation test adjustments. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@361405 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/CodeGen/TargetInstrInfo.h | 5 +-- include/llvm/MC/MCAsmInfo.h | 9 ++++- lib/CodeGen/TargetInstrInfo.cpp | 8 +++-- .../Disassembler/AMDGPUDisassembler.cpp | 12 ++++--- .../AMDGPU/Disassembler/AMDGPUDisassembler.h | 5 ++- .../AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.cpp | 22 ++++++++++++- .../AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.h | 1 + lib/Target/AMDGPU/SIInstrInfo.cpp | 3 +- lib/Target/Hexagon/HexagonInstrInfo.cpp | 6 ++-- lib/Target/Hexagon/HexagonInstrInfo.h | 6 ++-- .../branch-relaxation-inst-size-gfx10.ll | 33 +++++++++++++++++++ 11 files changed, 91 insertions(+), 19 deletions(-) create mode 100644 test/CodeGen/AMDGPU/branch-relaxation-inst-size-gfx10.ll diff --git a/include/llvm/CodeGen/TargetInstrInfo.h b/include/llvm/CodeGen/TargetInstrInfo.h index 3ba4d406461..efc2e3b776d 100644 --- a/include/llvm/CodeGen/TargetInstrInfo.h +++ b/include/llvm/CodeGen/TargetInstrInfo.h @@ -1260,8 +1260,9 @@ public: /// Measure the specified inline asm to determine an approximation of its /// length. - virtual unsigned getInlineAsmLength(const char *Str, - const MCAsmInfo &MAI) const; + virtual unsigned getInlineAsmLength( + const char *Str, const MCAsmInfo &MAI, + const TargetSubtargetInfo *STI = nullptr) const; /// Allocate and return a hazard recognizer to use for this target when /// scheduling the machine instructions before register allocation. diff --git a/include/llvm/MC/MCAsmInfo.h b/include/llvm/MC/MCAsmInfo.h index 9548674e70c..971e9354da8 100644 --- a/include/llvm/MC/MCAsmInfo.h +++ b/include/llvm/MC/MCAsmInfo.h @@ -27,6 +27,7 @@ class MCCFIInstruction; class MCExpr; class MCSection; class MCStreamer; +class MCSubtargetInfo; class MCSymbol; namespace WinEH { @@ -473,7 +474,13 @@ public: bool hasMachoTBSSDirective() const { return HasMachoTBSSDirective; } bool hasCOFFAssociativeComdats() const { return HasCOFFAssociativeComdats; } bool hasCOFFComdatConstants() const { return HasCOFFComdatConstants; } - unsigned getMaxInstLength() const { return MaxInstLength; } + + /// Returns the maximum possible encoded instruction size in bytes. If \p STI + /// is null, this should be the maximum size for any subtarget. + virtual unsigned getMaxInstLength(const MCSubtargetInfo *STI = nullptr) const { + return MaxInstLength; + } + unsigned getMinInstAlignment() const { return MinInstAlignment; } bool getDollarIsPC() const { return DollarIsPC; } const char *getSeparatorString() const { return SeparatorString; } diff --git a/lib/CodeGen/TargetInstrInfo.cpp b/lib/CodeGen/TargetInstrInfo.cpp index d397c3334b8..ab13d3482c1 100644 --- a/lib/CodeGen/TargetInstrInfo.cpp +++ b/lib/CodeGen/TargetInstrInfo.cpp @@ -85,11 +85,13 @@ static bool isAsmComment(const char *Str, const MCAsmInfo &MAI) { /// simple--i.e. not a logical or arithmetic expression--size values without /// the optional fill value. This is primarily used for creating arbitrary /// sized inline asm blocks for testing purposes. -unsigned TargetInstrInfo::getInlineAsmLength(const char *Str, - const MCAsmInfo &MAI) const { +unsigned TargetInstrInfo::getInlineAsmLength( + const char *Str, + const MCAsmInfo &MAI, const TargetSubtargetInfo *STI) const { // Count the number of instructions in the asm. bool AtInsnStart = true; unsigned Length = 0; + const unsigned MaxInstLength = MAI.getMaxInstLength(STI); for (; *Str; ++Str) { if (*Str == '\n' || strncmp(Str, MAI.getSeparatorString(), strlen(MAI.getSeparatorString())) == 0) { @@ -101,7 +103,7 @@ unsigned TargetInstrInfo::getInlineAsmLength(const char *Str, } if (AtInsnStart && !std::isspace(static_cast(*Str))) { - unsigned AddLength = MAI.getMaxInstLength(); + unsigned AddLength = MaxInstLength; if (strncmp(Str, ".space", 6) == 0) { char *EStr; int SpaceSize; diff --git a/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp b/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp index ea01416cd15..7db736766d2 100644 --- a/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp +++ b/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/Twine.h" #include "llvm/BinaryFormat/ELF.h" +#include "llvm/MC/MCAsmInfo.h" #include "llvm/MC/MCContext.h" #include "llvm/MC/MCDisassembler/MCDisassembler.h" #include "llvm/MC/MCExpr.h" @@ -56,6 +57,12 @@ using namespace llvm; using DecodeStatus = llvm::MCDisassembler::DecodeStatus; +AMDGPUDisassembler::AMDGPUDisassembler(const MCSubtargetInfo &STI, + MCContext &Ctx, + MCInstrInfo const *MCII) : + MCDisassembler(STI, Ctx), MCII(MCII), MRI(*Ctx.getRegisterInfo()), + TargetMaxInstBytes(Ctx.getAsmInfo()->getMaxInstLength(&STI)) {} + inline static MCDisassembler::DecodeStatus addOperand(MCInst &Inst, const MCOperand& Opnd) { Inst.addOperand(Opnd); @@ -186,10 +193,7 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size, if (!STI.getFeatureBits()[AMDGPU::FeatureGCN3Encoding] && !isGFX10()) report_fatal_error("Disassembly not yet supported for subtarget"); - unsigned MaxInstBytesNum = (std::min)( - STI.getFeatureBits()[AMDGPU::FeatureGFX10] ? (size_t) 20 : - STI.getFeatureBits()[AMDGPU::FeatureVOP3Literal] ? (size_t) 12 : (size_t)8, - Bytes_.size()); + unsigned MaxInstBytesNum = std::min((size_t)TargetMaxInstBytes, Bytes_.size()); Bytes = Bytes_.slice(0, MaxInstBytesNum); DecodeStatus Res = MCDisassembler::Fail; diff --git a/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h b/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h index 63cadd16303..3467d330dd8 100644 --- a/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h +++ b/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h @@ -41,15 +41,14 @@ class AMDGPUDisassembler : public MCDisassembler { private: std::unique_ptr const MCII; const MCRegisterInfo &MRI; + const unsigned TargetMaxInstBytes; mutable ArrayRef Bytes; mutable uint32_t Literal; mutable bool HasLiteral; public: AMDGPUDisassembler(const MCSubtargetInfo &STI, MCContext &Ctx, - MCInstrInfo const *MCII) : - MCDisassembler(STI, Ctx), MCII(MCII), MRI(*Ctx.getRegisterInfo()) {} - + MCInstrInfo const *MCII); ~AMDGPUDisassembler() override = default; DecodeStatus getInstruction(MCInst &MI, uint64_t &Size, diff --git a/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.cpp b/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.cpp index 2f3dbdc96f7..9e04ab9bae9 100644 --- a/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.cpp +++ b/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.cpp @@ -9,6 +9,8 @@ #include "AMDGPUMCAsmInfo.h" #include "llvm/ADT/Triple.h" +#include "llvm/MC/MCSubtargetInfo.h" +#include "MCTargetDesc/AMDGPUMCTargetDesc.h" using namespace llvm; @@ -18,7 +20,10 @@ AMDGPUMCAsmInfo::AMDGPUMCAsmInfo(const Triple &TT) : MCAsmInfoELF() { HasSingleParameterDotFile = false; //===------------------------------------------------------------------===// MinInstAlignment = 4; - MaxInstLength = (TT.getArch() == Triple::amdgcn) ? 8 : 16; + + // This is the maximum instruction encoded size for gfx10. With a known + // subtarget, it can be reduced to 8 bytes. + MaxInstLength = (TT.getArch() == Triple::amdgcn) ? 20 : 16; SeparatorString = "\n"; CommentString = ";"; PrivateLabelPrefix = ""; @@ -44,3 +49,18 @@ bool AMDGPUMCAsmInfo::shouldOmitSectionDirective(StringRef SectionName) const { SectionName == ".hsarodata_readonly_agent" || MCAsmInfo::shouldOmitSectionDirective(SectionName); } + +unsigned AMDGPUMCAsmInfo::getMaxInstLength(const MCSubtargetInfo *STI) const { + if (!STI || STI->getTargetTriple().getArch() == Triple::r600) + return MaxInstLength; + + // Maximum for NSA encoded images + if (STI->getFeatureBits()[AMDGPU::FeatureNSAEncoding]) + return 20; + + // 64-bit instruction with 32-bit literal. + if (STI->getFeatureBits()[AMDGPU::FeatureVOP3Literal]) + return 12; + + return 8; +} diff --git a/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.h b/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.h index a3c069ce970..71e63ec27a8 100644 --- a/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.h +++ b/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.h @@ -27,6 +27,7 @@ class AMDGPUMCAsmInfo : public MCAsmInfoELF { public: explicit AMDGPUMCAsmInfo(const Triple &TT); bool shouldOmitSectionDirective(StringRef SectionName) const override; + unsigned getMaxInstLength(const MCSubtargetInfo *STI) const override; }; } // namespace llvm #endif diff --git a/lib/Target/AMDGPU/SIInstrInfo.cpp b/lib/Target/AMDGPU/SIInstrInfo.cpp index e781253dd01..e42ed3505cf 100644 --- a/lib/Target/AMDGPU/SIInstrInfo.cpp +++ b/lib/Target/AMDGPU/SIInstrInfo.cpp @@ -5578,7 +5578,8 @@ unsigned SIInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const { case TargetOpcode::INLINEASM_BR: { const MachineFunction *MF = MI.getParent()->getParent(); const char *AsmStr = MI.getOperand(0).getSymbolName(); - return getInlineAsmLength(AsmStr, *MF->getTarget().getMCAsmInfo()); + return getInlineAsmLength(AsmStr, *MF->getTarget().getMCAsmInfo(), + &MF->getSubtarget()); } default: return DescSize; diff --git a/lib/Target/Hexagon/HexagonInstrInfo.cpp b/lib/Target/Hexagon/HexagonInstrInfo.cpp index 545dd15dde2..d525baa0301 100644 --- a/lib/Target/Hexagon/HexagonInstrInfo.cpp +++ b/lib/Target/Hexagon/HexagonInstrInfo.cpp @@ -1712,17 +1712,19 @@ bool HexagonInstrInfo::isSchedulingBoundary(const MachineInstr &MI, /// Hexagon counts the number of ##'s and adjust for that many /// constant exenders. unsigned HexagonInstrInfo::getInlineAsmLength(const char *Str, - const MCAsmInfo &MAI) const { + const MCAsmInfo &MAI, + const TargetSubtargetInfo *STI) const { StringRef AStr(Str); // Count the number of instructions in the asm. bool atInsnStart = true; unsigned Length = 0; + const unsigned MaxInstLength = MAI.getMaxInstLength(STI); for (; *Str; ++Str) { if (*Str == '\n' || strncmp(Str, MAI.getSeparatorString(), strlen(MAI.getSeparatorString())) == 0) atInsnStart = true; if (atInsnStart && !std::isspace(static_cast(*Str))) { - Length += MAI.getMaxInstLength(); + Length += MaxInstLength; atInsnStart = false; } if (atInsnStart && strncmp(Str, MAI.getCommentString().data(), diff --git a/lib/Target/Hexagon/HexagonInstrInfo.h b/lib/Target/Hexagon/HexagonInstrInfo.h index 8d339641ba4..6c4dbfedb31 100644 --- a/lib/Target/Hexagon/HexagonInstrInfo.h +++ b/lib/Target/Hexagon/HexagonInstrInfo.h @@ -264,8 +264,10 @@ public: /// Measure the specified inline asm to determine an approximation of its /// length. - unsigned getInlineAsmLength(const char *Str, - const MCAsmInfo &MAI) const override; + unsigned getInlineAsmLength( + const char *Str, + const MCAsmInfo &MAI, + const TargetSubtargetInfo *STI = nullptr) const override; /// Allocate and return a hazard recognizer to use for this target when /// scheduling the machine instructions after register allocation. diff --git a/test/CodeGen/AMDGPU/branch-relaxation-inst-size-gfx10.ll b/test/CodeGen/AMDGPU/branch-relaxation-inst-size-gfx10.ll new file mode 100644 index 00000000000..d40a6de6318 --- /dev/null +++ b/test/CodeGen/AMDGPU/branch-relaxation-inst-size-gfx10.ll @@ -0,0 +1,33 @@ +; RUN: llc -march=amdgcn -mcpu=gfx1010 -verify-machineinstrs -amdgpu-s-branch-bits=4 < %s | FileCheck -enable-var-scope -check-prefixes=GCN,GFX10 %s +; RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs -amdgpu-s-branch-bits=4 < %s | FileCheck -enable-var-scope -check-prefixes=GCN,GFX9 %s + +; Make sure the code size estimate for inline asm is 12-bytes per +; instruction, rather than 8 in previous generations. + +; GCN-LABEL: {{^}}long_forward_branch_gfx10only: +; GFX9: s_cmp_eq_u32 +; GFX9-NEXT: s_cbranch_scc1 + +; GFX10: s_cmp_eq_u32 +; GFX10-NEXT: s_cbranch_scc0 +; GFX10: s_getpc_b64 +; GFX10: s_add_u32 +; GFX10: s_addc_u32 +; GFX10: s_setpc_b64 +define amdgpu_kernel void @long_forward_branch_gfx10only(i32 addrspace(1)* %arg, i32 %cnd) #0 { +bb0: + %cmp = icmp eq i32 %cnd, 0 + br i1 %cmp, label %bb3, label %bb2 ; +9 dword branch + +bb2: + ; Estimated as 40-bytes on gfx10 (requiring a long branch), but + ; 16-bytes on gfx9 (allowing a short branch) + call void asm sideeffect + "v_nop_e64 + v_nop_e64", ""() #0 + br label %bb3 + +bb3: + store volatile i32 %cnd, i32 addrspace(1)* %arg + ret void +} -- 2.40.0