From 5f209637c4a6297315774c3e7098cc1f458e80f7 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Wed, 3 Sep 2014 22:46:44 +0000 Subject: [PATCH] [x86] Teach the asm comment printing to only print the clarification of an immediate operand when we don't have instruction-specific comments. This ensures that instruction-specific comments are attached to the same line as the instruction which is important for using them to write readable and maintainable tests. My next commit will just such a test. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@217099 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../X86/InstPrinter/X86ATTInstPrinter.cpp | 15 ++- .../X86/InstPrinter/X86ATTInstPrinter.h | 3 + .../X86/InstPrinter/X86InstComments.cpp | 93 ++++++++++--------- lib/Target/X86/InstPrinter/X86InstComments.h | 2 +- 4 files changed, 64 insertions(+), 49 deletions(-) diff --git a/lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp b/lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp index b45b1185bd6..d74a19e4dbc 100644 --- a/lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp +++ b/lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp @@ -45,6 +45,11 @@ void X86ATTInstPrinter::printInst(const MCInst *MI, raw_ostream &OS, const MCInstrDesc &Desc = MII.get(MI->getOpcode()); uint64_t TSFlags = Desc.TSFlags; + // If verbose assembly is enabled, we can print some informative comments. + if (CommentStream) + HasCustomInstComment = + EmitAnyX86InstComments(MI, *CommentStream, getRegisterName); + if (TSFlags & X86II::LOCK) OS << "\tlock\n"; @@ -54,10 +59,6 @@ void X86ATTInstPrinter::printInst(const MCInst *MI, raw_ostream &OS, // Next always print the annotation. printAnnotation(OS, Annot); - - // If verbose assembly is enabled, we can print some informative comments. - if (CommentStream) - EmitAnyX86InstComments(MI, *CommentStream, getRegisterName); } void X86ATTInstPrinter::printSSECC(const MCInst *MI, unsigned Op, @@ -170,7 +171,11 @@ void X86ATTInstPrinter::printOperand(const MCInst *MI, unsigned OpNo, << '$' << formatImm((int64_t)Op.getImm()) << markup(">"); - if (CommentStream && (Op.getImm() > 255 || Op.getImm() < -256)) + // If there are no instruction-specific comments, add a comment clarifying + // the hex value of the immediate operand when it isn't in the range + // [-256,255]. + if (CommentStream && !HasCustomInstComment && + (Op.getImm() > 255 || Op.getImm() < -256)) *CommentStream << format("imm = 0x%" PRIX64 "\n", (uint64_t)Op.getImm()); } else { diff --git a/lib/Target/X86/InstPrinter/X86ATTInstPrinter.h b/lib/Target/X86/InstPrinter/X86ATTInstPrinter.h index d2c0b90489e..175892db057 100644 --- a/lib/Target/X86/InstPrinter/X86ATTInstPrinter.h +++ b/lib/Target/X86/InstPrinter/X86ATTInstPrinter.h @@ -129,6 +129,9 @@ public: void printMemOffs64(const MCInst *MI, unsigned OpNo, raw_ostream &O) { printMemOffset(MI, OpNo, O); } + +private: + bool HasCustomInstComment; }; } diff --git a/lib/Target/X86/InstPrinter/X86InstComments.cpp b/lib/Target/X86/InstPrinter/X86InstComments.cpp index 8685e48b640..5deb6825148 100644 --- a/lib/Target/X86/InstPrinter/X86InstComments.cpp +++ b/lib/Target/X86/InstPrinter/X86InstComments.cpp @@ -28,13 +28,17 @@ using namespace llvm; /// EmitAnyX86InstComments - This function decodes x86 instructions and prints /// newline terminated strings to the specified string if desired. This /// information is shown in disassembly dumps when verbose assembly is enabled. -void llvm::EmitAnyX86InstComments(const MCInst *MI, raw_ostream &OS, +bool llvm::EmitAnyX86InstComments(const MCInst *MI, raw_ostream &OS, const char *(*getRegName)(unsigned)) { // If this is a shuffle operation, the switch should fill in this state. SmallVector ShuffleMask; const char *DestName = nullptr, *Src1Name = nullptr, *Src2Name = nullptr; switch (MI->getOpcode()) { + default: + // Not an instruction for which we can decode comments. + return false; + case X86::BLENDPDrri: case X86::VBLENDPDrri: Src2Name = getRegName(MI->getOperand(2).getReg()); @@ -553,54 +557,57 @@ void llvm::EmitAnyX86InstComments(const MCInst *MI, raw_ostream &OS, break; } + // The only comments we decode are shuffles, so give up if we were unable to + // decode a shuffle mask. + if (ShuffleMask.empty()) + return false; - // If this was a shuffle operation, print the shuffle mask. - if (!ShuffleMask.empty()) { - if (!DestName) DestName = Src1Name; - OS << (DestName ? DestName : "mem") << " = "; + if (!DestName) DestName = Src1Name; + OS << (DestName ? DestName : "mem") << " = "; - // If the two sources are the same, canonicalize the input elements to be - // from the first src so that we get larger element spans. - if (Src1Name == Src2Name) { - for (unsigned i = 0, e = ShuffleMask.size(); i != e; ++i) { - if ((int)ShuffleMask[i] >= 0 && // Not sentinel. - ShuffleMask[i] >= (int)e) // From second mask. - ShuffleMask[i] -= e; - } + // If the two sources are the same, canonicalize the input elements to be + // from the first src so that we get larger element spans. + if (Src1Name == Src2Name) { + for (unsigned i = 0, e = ShuffleMask.size(); i != e; ++i) { + if ((int)ShuffleMask[i] >= 0 && // Not sentinel. + ShuffleMask[i] >= (int)e) // From second mask. + ShuffleMask[i] -= e; } + } - // The shuffle mask specifies which elements of the src1/src2 fill in the - // destination, with a few sentinel values. Loop through and print them - // out. - for (unsigned i = 0, e = ShuffleMask.size(); i != e; ++i) { - if (i != 0) - OS << ','; - if (ShuffleMask[i] == SM_SentinelZero) { - OS << "zero"; - continue; - } + // The shuffle mask specifies which elements of the src1/src2 fill in the + // destination, with a few sentinel values. Loop through and print them + // out. + for (unsigned i = 0, e = ShuffleMask.size(); i != e; ++i) { + if (i != 0) + OS << ','; + if (ShuffleMask[i] == SM_SentinelZero) { + OS << "zero"; + continue; + } - // Otherwise, it must come from src1 or src2. Print the span of elements - // that comes from this src. - bool isSrc1 = ShuffleMask[i] < (int)ShuffleMask.size(); - const char *SrcName = isSrc1 ? Src1Name : Src2Name; - OS << (SrcName ? SrcName : "mem") << '['; - bool IsFirst = true; - while (i != e && - (int)ShuffleMask[i] >= 0 && - (ShuffleMask[i] < (int)ShuffleMask.size()) == isSrc1) { - if (!IsFirst) - OS << ','; - else - IsFirst = false; - OS << ShuffleMask[i] % ShuffleMask.size(); - ++i; - } - OS << ']'; - --i; // For loop increments element #. + // Otherwise, it must come from src1 or src2. Print the span of elements + // that comes from this src. + bool isSrc1 = ShuffleMask[i] < (int)ShuffleMask.size(); + const char *SrcName = isSrc1 ? Src1Name : Src2Name; + OS << (SrcName ? SrcName : "mem") << '['; + bool IsFirst = true; + while (i != e && + (int)ShuffleMask[i] >= 0 && + (ShuffleMask[i] < (int)ShuffleMask.size()) == isSrc1) { + if (!IsFirst) + OS << ','; + else + IsFirst = false; + OS << ShuffleMask[i] % ShuffleMask.size(); + ++i; } - //MI->print(OS, 0); - OS << "\n"; + OS << ']'; + --i; // For loop increments element #. } + //MI->print(OS, 0); + OS << "\n"; + // We successfully added a comment to this instruction. + return true; } diff --git a/lib/Target/X86/InstPrinter/X86InstComments.h b/lib/Target/X86/InstPrinter/X86InstComments.h index 7f4122dd381..687581b10ae 100644 --- a/lib/Target/X86/InstPrinter/X86InstComments.h +++ b/lib/Target/X86/InstPrinter/X86InstComments.h @@ -18,7 +18,7 @@ namespace llvm { class MCInst; class raw_ostream; - void EmitAnyX86InstComments(const MCInst *MI, raw_ostream &OS, + bool EmitAnyX86InstComments(const MCInst *MI, raw_ostream &OS, const char *(*getRegName)(unsigned)); } -- 2.34.1