[llvm] r314393 - Teach TargetInstrInfo::getInlineAsmLength to parse .space directives with integer arguments

Alex Bradbury via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 28 02:31:46 PDT 2017


Author: asb
Date: Thu Sep 28 02:31:46 2017
New Revision: 314393

URL: http://llvm.org/viewvc/llvm-project?rev=314393&view=rev
Log:
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


Modified:
    llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp
    llvm/trunk/lib/Target/Mips/Mips16InstrInfo.cpp
    llvm/trunk/lib/Target/Mips/Mips16InstrInfo.h

Modified: llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp?rev=314393&r1=314392&r2=314393&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp (original)
+++ llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp Thu Sep 28 02:31:46 2017
@@ -67,6 +67,11 @@ void TargetInstrInfo::insertNoop(Machine
   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(Machine
 /// 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

Modified: llvm/trunk/lib/Target/Mips/Mips16InstrInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/Mips16InstrInfo.cpp?rev=314393&r1=314392&r2=314393&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Mips/Mips16InstrInfo.cpp (original)
+++ llvm/trunk/lib/Target/Mips/Mips16InstrInfo.cpp Thu Sep 28 02:31:46 2017
@@ -483,45 +483,3 @@ bool Mips16InstrInfo::validImmediate(uns
   }
   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;
-}

Modified: llvm/trunk/lib/Target/Mips/Mips16InstrInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/Mips16InstrInfo.h?rev=314393&r1=314392&r2=314393&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Mips/Mips16InstrInfo.h (original)
+++ llvm/trunk/lib/Target/Mips/Mips16InstrInfo.h Thu Sep 28 02:31:46 2017
@@ -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;
 




More information about the llvm-commits mailing list