Teach TargetInstrInfo::getInlineAsmLength to parse .space directives with integer arguments

It's currently quite difficult to test passes like branch relaxation, which
requires branches with large displacement to be generated. The .space assembler
directive makes it easy to create arbitrarily large basic blocks, but
getInlineAsmLength is not able to parse it and so the size of the block is not
correctly estimated. Other backends (AArch64, AMDGPU) introduce options just
for testing that artificially restrict the ranges of branch instructions (e.g.
aarch64-tbz-offset-bits). Although parsing a single form of the .space
directive feels inelegant, it does allow a more direct testing approach.

This patch adapts the .space parsing code from
Mips16InstrInfo::getInlineAsmLength and removes it now the extra functionality
is provided by the base implementation. I want to move this functionality to
the generic getInlineAsmLength as 1) I need the same for RISC-V, and 2) I feel
other backends will benefit from more direct testing of large branch
displacements.

Differential Revision: https://reviews.llvm.org/D37798



git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@314393 91177308-0d34-0410-b5e6-96231b3b80d8

lib/CodeGen/TargetInstrInfo.cpp

@@ -67,6 +67,11 @@ void TargetInstrInfo::insertNoop(MachineBasicBlock &MBB,
   llvm_unreachable("Target didn't implement insertNoop!");
 }
 
+static bool isAsmComment(const char *Str, const MCAsmInfo &MAI) {
+  return strncmp(Str, MAI.getCommentString().data(),
+                 MAI.getCommentString().size()) == 0;
+}
+
 /// Measure the specified inline asm to determine an approximation of its
 /// length.
 /// Comments (which run till the next SeparatorString or newline) do not
@@ -75,29 +80,46 @@ void TargetInstrInfo::insertNoop(MachineBasicBlock &MBB,
 /// 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.
+/// 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 {
   // Count the number of instructions in the asm.
-  bool atInsnStart = true;
-  unsigned InstCount = 0;
+  bool AtInsnStart = true;
+  unsigned Length = 0;
   for (; *Str; ++Str) {
     if (*Str == '\n' || strncmp(Str, MAI.getSeparatorString(),
                                 strlen(MAI.getSeparatorString())) == 0) {
-      atInsnStart = true;
-    } else if (strncmp(Str, MAI.getCommentString().data(),
-                       MAI.getCommentString().size()) == 0) {
+      AtInsnStart = true;
+    } else if (isAsmComment(Str, MAI)) {
       // Stop counting as an instruction after a comment until the next
       // separator.
-      atInsnStart = false;
+      AtInsnStart = false;
     }
 
-    if (atInsnStart && !std::isspace(static_cast<unsigned char>(*Str))) {
-      ++InstCount;
-      atInsnStart = false;
+    if (AtInsnStart && !std::isspace(static_cast<unsigned char>(*Str))) {
+      unsigned AddLength = MAI.getMaxInstLength();
+      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' ||
+            isAsmComment(EStr, MAI)) // Successfully parsed .space argument
+          AddLength = SpaceSize;
+      }
+      Length += AddLength;
+      AtInsnStart = false;
     }
   }
 
-  return InstCount * MAI.getMaxInstLength();
+  return Length;
 }
 
 /// ReplaceTailWithBranchTo - Delete the instruction OldInst and everything

lib/Target/Mips/Mips16InstrInfo.cpp

@@ -483,45 +483,3 @@ bool Mips16InstrInfo::validImmediate(unsigned Opcode, unsigned Reg,
   }
   llvm_unreachable("unexpected Opcode in validImmediate");
 }
-
-/// Measure the specified inline asm to determine an approximation of its
-/// length.
-/// Comments (which run till the next SeparatorString or newline) do not
-/// 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.
-/// We implement the special case of the .space directive taking only an
-/// integer argument, which is the size in bytes. This is used for creating
-/// inline code spacing for testing purposes using inline assembly.
-unsigned Mips16InstrInfo::getInlineAsmLength(const char *Str,
-                                             const MCAsmInfo &MAI) const {
-
-  // Count the number of instructions in the asm.
-  bool atInsnStart = true;
-  unsigned Length = 0;
-  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))) {
-      if (strncmp(Str, ".space", 6)==0) {
-        char *EStr; int Sz;
-        Sz = strtol(Str+6, &EStr, 10);
-        while (isspace(*EStr)) ++EStr;
-        if (*EStr=='\0') {
-          DEBUG(dbgs() << "parsed .space " << Sz << '\n');
-          return Sz;
-        }
-      }
-      Length += MAI.getMaxInstLength();
-      atInsnStart = false;
-    }
-    if (atInsnStart && strncmp(Str, MAI.getCommentString().data(),
-                               MAI.getCommentString().size()) == 0)
-      atInsnStart = false;
-  }
-
-  return Length;
-}

lib/Target/Mips/Mips16InstrInfo.h

@@ -102,9 +102,6 @@ public:
 
   void BuildAddiuSpImm
     (MachineBasicBlock &MBB, MachineBasicBlock::iterator I, int64_t Imm) const;
-
-  unsigned getInlineAsmLength(const char *Str,
-                              const MCAsmInfo &MAI) const override;
 private:
   unsigned getAnalyzableBrOpc(unsigned Opc) const override;