MC: Allow getMaxInstLength to depend on the subtarget

[arsenm]

Summary

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
12-bytes. This avoids penalizing older subtargets when estimating code
size, and making some annoying branch relaxation test adjustments.

[rampitec]

lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.cpp

26

I believe it is 20 though:

1. GFX10: image_sample_c_d v[16:19], [v8, v9, v10, v11, v12, v13, v14, v15, v16, v17], s[20:27], s[100:103] dmask:0xf dim:SQ_RSRC_IMG_3D ; encoding: [0x16,0x0f,0xa8,0xf0,0x08,0x10,0x25,0x03,0x09,0x0a,0x0b,0x0c,0x0d,0x0e,0x0f,0x10,0x11,0x00,0x00,0x00]

[arsenm]

Increase to 20

[rampitec]

LGTM

[arsenm]

Avoid duplicating in the disassembler

[arsenm]

Use NSA feature

[rampitec]

LGTM

[rampitec]

JFYI, you need to mention nsa in the commit description.

[arsenm]

r361405

include/llvm/CodeGen/TargetInstrInfo.h

public:
   /// Test if the given instruction should be considered a scheduling boundary.
   /// This primarily includes labels and terminators.
   virtual bool isSchedulingBoundary(const MachineInstr &MI,
                                     const MachineBasicBlock *MBB,
                                     const MachineFunction &MF) const;
 
   /// 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.
   virtual ScheduleHazardRecognizer *
   CreateTargetHazardRecognizer(const TargetSubtargetInfo *STI,
                                const ScheduleDAG *DAG) const;
 
   /// Allocate and return a hazard recognizer to use for this target when

include/llvm/MC/MCAsmInfo.h

 
 namespace llvm {
 
 class MCContext;
 class MCCFIInstruction;
 class MCExpr;
 class MCSection;
 class MCStreamer;
+class MCSubtargetInfo;
 class MCSymbol;
 
 namespace WinEH {
 
 enum class EncodingType {
   Invalid, /// Invalid
   Alpha,   /// Windows Alpha
   Alpha64, /// Windows AXP64
public:
   }
 
   // Accessors.
 
   bool hasMachoZeroFillDirective() const { return HasMachoZeroFillDirective; }
   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; }
 
   /// This indicates the column (zero-based) at which asm comments should be
   /// printed.
   unsigned getCommentColumn() const { return 40; }
 

lib/CodeGen/TargetInstrInfo.cpp

 /// Variable-length instructions are not handled here; this function
 /// may be overloaded in the target code to do that.
 /// We implement a special case of the .space directive which takes only a
 /// single integer argument in base 10 that is the size in bytes. This is a
 /// restricted form of the GAS directive in that we only interpret
 /// 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) {
       AtInsnStart = true;
     } else if (isAsmComment(Str, MAI)) {
       // Stop counting as an instruction after a comment until the next
       // separator.
       AtInsnStart = false;
     }
 
     if (AtInsnStart && !std::isspace(static_cast<unsigned char>(*Str))) {
-      unsigned AddLength = MAI.getMaxInstLength();
+      unsigned AddLength = MaxInstLength;
       if (strncmp(Str, ".space", 6) == 0) {
         char *EStr;
         int SpaceSize;
         SpaceSize = strtol(Str + 6, &EStr, 10);
         SpaceSize = SpaceSize < 0 ? 0 : SpaceSize;
         while (*EStr != '\n' && std::isspace(static_cast<unsigned char>(*EStr)))
           ++EStr;
         if (*EStr == '\0' || *EStr == '\n' ||

lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h

 //===----------------------------------------------------------------------===//
 // AMDGPUDisassembler
 //===----------------------------------------------------------------------===//
 
 class AMDGPUDisassembler : public MCDisassembler {
 private:
   std::unique_ptr<MCInstrInfo const> const MCII;
   const MCRegisterInfo &MRI;
+  const unsigned TargetMaxInstBytes;
   mutable ArrayRef<uint8_t> 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,
                               ArrayRef<uint8_t> Bytes, uint64_t Address,
                               raw_ostream &WS, raw_ostream &CS) const override;
 
   const char* getRegClassName(unsigned RegClassID) const;
 

lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp

 #include "SIDefines.h"
 #include "TargetInfo/AMDGPUTargetInfo.h"
 #include "Utils/AMDGPUBaseInfo.h"
 #include "llvm-c/Disassembler.h"
 #include "llvm/ADT/APInt.h"
 #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"
 #include "llvm/MC/MCFixedLenDisassembler.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/ErrorHandling.h"

 
 #define DEBUG_TYPE "amdgpu-disassembler"
 
 #define SGPR_MAX (isGFX10() ? AMDGPU::EncValues::SGPR_MAX_GFX10 \
                             : AMDGPU::EncValues::SGPR_MAX_SI)
 
 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);
   return Opnd.isValid() ?
     MCDisassembler::Success :
     MCDisassembler::SoftFail;
 }
 
DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
                                                 raw_ostream &CS) const {
   CommentStream = &CS;
   bool IsSDWA = false;
 
   // ToDo: AMDGPUDisassembler supports only VI ISA.
   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;
   do {
     // ToDo: better to switch encoding length using some bit predicate
     // but it is unknown yet, so try all we can
 
     // Try to decode DPP and SDWA first to solve conflict with VOP1 and VOP2

lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.h

 // you will need to make sure your new class sets PrivateGlobalPrefix to
 // a prefix that won't appear in a function name.  The default value
 // for PrivateGlobalPrefix is 'L', so it will consider any function starting
 // with 'L' as a local symbol.
 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

lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.cpp

 //===-- MCTargetDesc/AMDGPUMCAsmInfo.cpp - Assembly Info ------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 /// \file
 //===----------------------------------------------------------------------===//
 
 #include "AMDGPUMCAsmInfo.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/MC/MCSubtargetInfo.h"
+#include "MCTargetDesc/AMDGPUMCTargetDesc.h"
 
 using namespace llvm;
 
 AMDGPUMCAsmInfo::AMDGPUMCAsmInfo(const Triple &TT) : MCAsmInfoELF() {
   CodePointerSize = (TT.getArch() == Triple::amdgcn) ? 8 : 4;
   StackGrowsUp = true;
   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 = "";
   InlineAsmStart = ";#ASMSTART";
   InlineAsmEnd = ";#ASMEND";
 
   //===--- Data Emission Directives -------------------------------------===//
   SunStyleELFSectionSwitchSyntax = true;

 }
 
 bool AMDGPUMCAsmInfo::shouldOmitSectionDirective(StringRef SectionName) const {
   return SectionName == ".hsatext" || SectionName == ".hsadata_global_agent" ||
          SectionName == ".hsadata_global_program" ||
          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;
+}

lib/Target/AMDGPU/SIInstrInfo.cpp

unsigned SIInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
   case TargetOpcode::EH_LABEL:
     return 0;
   case TargetOpcode::BUNDLE:
     return getInstBundleSize(MI);
   case TargetOpcode::INLINEASM:
   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;
   }
 }
 
 bool SIInstrInfo::mayAccessFlatAddressSpace(const MachineInstr &MI) const {
   if (!isFLAT(MI))

lib/Target/Hexagon/HexagonInstrInfo.h

public:
   /// Test if the given instruction should be considered a scheduling boundary.
   /// This primarily includes labels and terminators.
   bool isSchedulingBoundary(const MachineInstr &MI,
                             const MachineBasicBlock *MBB,
                             const MachineFunction &MF) const override;
 
   /// 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.
   ScheduleHazardRecognizer*
   CreateTargetPostRAHazardRecognizer(const InstrItineraryData *II,
                                      const ScheduleDAG *DAG) const override;
 
   /// For a comparison instruction, return the source registers

lib/Target/Hexagon/HexagonInstrInfo.cpp

 /// count as an instruction.
 /// Any other non-whitespace text is considered an instruction, with
 /// multiple instructions separated by SeparatorString or newlines.
 /// Variable-length instructions are not handled here; this function
 /// may be overloaded in the target code to do that.
 /// 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<unsigned char>(*Str))) {
-      Length += MAI.getMaxInstLength();
+      Length += MaxInstLength;
       atInsnStart = false;
     }
     if (atInsnStart && strncmp(Str, MAI.getCommentString().data(),
                                MAI.getCommentString().size()) == 0)
       atInsnStart = false;
   }
 
   // Add to size number of constant extenders seen * 4.

test/CodeGen/AMDGPU/branch-relaxation-inst-size-gfx10.ll

+; 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
+}