[llvm] r194575 - Remove AllowQuotesInName and friends from MCAsmInfo.
Eli Bendersky
eliben at google.com
Fri Mar 7 13:56:11 PST 2014
On Wed, Nov 13, 2013 at 6:01 AM, Rafael Espindola <
rafael.espindola at gmail.com> wrote:
> Author: rafael
> Date: Wed Nov 13 08:01:59 2013
> New Revision: 194575
>
> URL: http://llvm.org/viewvc/llvm-project?rev=194575&view=rev
> Log:
> Remove AllowQuotesInName and friends from MCAsmInfo.
>
> Accepting quotes is a property of an assembler, not of an object file. For
> example, ELF can support any names for sections and symbols, but the gnu
> assembler only accepts quotes in some contexts and llvm-mc in a few more.
>
> LLVM should not produce different symbols based on a guess about which
> assembler
> will be reading the code it is printing.
>
> Modified:
> llvm/trunk/include/llvm/MC/MCAsmInfo.h
> llvm/trunk/lib/MC/MCAsmInfo.cpp
> llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp
> llvm/trunk/lib/MC/MCAsmInfoDarwin.cpp
> llvm/trunk/lib/MC/MCSectionELF.cpp
> llvm/trunk/lib/Target/MSP430/MCTargetDesc/MSP430MCAsmInfo.cpp
> llvm/trunk/lib/Target/Mangler.cpp
> llvm/trunk/lib/Target/NVPTX/MCTargetDesc/NVPTXMCAsmInfo.cpp
> llvm/trunk/lib/Target/R600/MCTargetDesc/AMDGPUMCAsmInfo.cpp
> llvm/trunk/test/CodeGen/X86/GC/ocaml-gc.ll
> llvm/trunk/test/CodeGen/X86/global-sections.ll
>
Hi Rafael,
Sorry to get back to this months after the commit, but I just ran into
problems with this. It appears to be a regression for NVPTX, which emits
assembly for the consumption of a non-standard assembler (ptxas, not gas or
llvm gas-like assemblers). Without this provision in place, LLVM emits
symbol names with periods, which ptxas chokes on.
Can you elaborate on the technical reason for this change a bit more?
Should LLVM only care about gas and llvm-mc as assemblers? What about
backends that have custom assemblers? How would you suggest to work around
this issue if reviving the AllowPeriodsInName flag is a no-go?
Thanks in advance,
Eli
>
> Modified: llvm/trunk/include/llvm/MC/MCAsmInfo.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAsmInfo.h?rev=194575&r1=194574&r2=194575&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/MCAsmInfo.h (original)
> +++ llvm/trunk/include/llvm/MC/MCAsmInfo.h Wed Nov 13 08:01:59 2013
> @@ -144,18 +144,6 @@ namespace llvm {
> /// AssemblerDialect - Which dialect of an assembler variant to use.
> unsigned AssemblerDialect; // Defaults to 0
>
> - /// AllowQuotesInName - This is true if the assembler allows for
> complex
> - /// symbol names to be surrounded in quotes. This defaults to false.
> - bool AllowQuotesInName;
> -
> - /// AllowNameToStartWithDigit - This is true if the assembler allows
> symbol
> - /// names to start with a digit (e.g., "0x0021"). This defaults to
> false.
> - bool AllowNameToStartWithDigit;
> -
> - /// AllowPeriodsInName - This is true if the assembler allows periods
> in
> - /// symbol names. This defaults to true.
> - bool AllowPeriodsInName;
> -
> /// \brief This is true if the assembler allows @ characters in symbol
> /// names. Defaults to false.
> bool AllowAtInName;
> @@ -467,15 +455,6 @@ namespace llvm {
> unsigned getAssemblerDialect() const {
> return AssemblerDialect;
> }
> - bool doesAllowQuotesInName() const {
> - return AllowQuotesInName;
> - }
> - bool doesAllowNameToStartWithDigit() const {
> - return AllowNameToStartWithDigit;
> - }
> - bool doesAllowPeriodsInName() const {
> - return AllowPeriodsInName;
> - }
> bool doesAllowAtInName() const {
> return AllowAtInName;
> }
>
> Modified: llvm/trunk/lib/MC/MCAsmInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmInfo.cpp?rev=194575&r1=194574&r2=194575&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/MC/MCAsmInfo.cpp (original)
> +++ llvm/trunk/lib/MC/MCAsmInfo.cpp Wed Nov 13 08:01:59 2013
> @@ -50,9 +50,6 @@ MCAsmInfo::MCAsmInfo() {
> Code32Directive = ".code32";
> Code64Directive = ".code64";
> AssemblerDialect = 0;
> - AllowQuotesInName = false;
> - AllowNameToStartWithDigit = false;
> - AllowPeriodsInName = true;
> AllowAtInName = false;
> UseDataRegionDirectives = false;
> ZeroDirective = "\t.zero\t";
>
> Modified: llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp?rev=194575&r1=194574&r2=194575&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp (original)
> +++ llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp Wed Nov 13 08:01:59 2013
> @@ -43,7 +43,6 @@ MCAsmInfoCOFF::MCAsmInfoCOFF() {
> void MCAsmInfoMicrosoft::anchor() { }
>
> MCAsmInfoMicrosoft::MCAsmInfoMicrosoft() {
> - AllowQuotesInName = true;
> }
>
> void MCAsmInfoGNUCOFF::anchor() { }
>
> Modified: llvm/trunk/lib/MC/MCAsmInfoDarwin.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmInfoDarwin.cpp?rev=194575&r1=194574&r2=194575&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/MC/MCAsmInfoDarwin.cpp (original)
> +++ llvm/trunk/lib/MC/MCAsmInfoDarwin.cpp Wed Nov 13 08:01:59 2013
> @@ -26,7 +26,6 @@ MCAsmInfoDarwin::MCAsmInfoDarwin() {
> GlobalPrefix = "_";
> PrivateGlobalPrefix = "L";
> LinkerPrivateGlobalPrefix = "l";
> - AllowQuotesInName = true;
> HasSingleParameterDotFile = false;
> HasSubsectionsViaSymbols = true;
>
>
> Modified: llvm/trunk/lib/MC/MCSectionELF.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCSectionELF.cpp?rev=194575&r1=194574&r2=194575&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/MC/MCSectionELF.cpp (original)
> +++ llvm/trunk/lib/MC/MCSectionELF.cpp Wed Nov 13 08:01:59 2013
> @@ -32,6 +32,29 @@ bool MCSectionELF::ShouldOmitSectionDire
> return false;
> }
>
> +static void printName(raw_ostream &OS, StringRef Name) {
> + if (Name.find_first_not_of("0123456789_."
> + "abcdefghijklmnopqrstuvwxyz"
> + "ABCDEFGHIJKLMNOPQRSTUVWXYZ") == Name.npos) {
> + OS << Name;
> + return;
> + }
> + OS << '"';
> + for (const char *B = Name.begin(), *E = Name.end(); B < E; ++B) {
> + if (*B == '"') // Unquoted "
> + OS << "\\\"";
> + else if (*B != '\\') // Neither " or backslash
> + OS << *B;
> + else if (B + 1 == E) // Trailing backslash
> + OS << "\\\\";
> + else {
> + OS << B[0] << B[1]; // Quoted character
> + ++B;
> + }
> + }
> + OS << '"';
> +}
> +
> void MCSectionELF::PrintSwitchToSection(const MCAsmInfo &MAI,
> raw_ostream &OS,
> const MCExpr *Subsection) const {
> @@ -44,27 +67,8 @@ void MCSectionELF::PrintSwitchToSection(
> return;
> }
>
> - StringRef name = getSectionName();
> - if (name.find_first_not_of("0123456789_."
> - "abcdefghijklmnopqrstuvwxyz"
> - "ABCDEFGHIJKLMNOPQRSTUVWXYZ") == name.npos) {
> - OS << "\t.section\t" << name;
> - } else {
> - OS << "\t.section\t\"";
> - for (const char *b = name.begin(), *e = name.end(); b < e; ++b) {
> - if (*b == '"') // Unquoted "
> - OS << "\\\"";
> - else if (*b != '\\') // Neither " or backslash
> - OS << *b;
> - else if (b + 1 == e) // Trailing backslash
> - OS << "\\\\";
> - else {
> - OS << b[0] << b[1]; // Quoted character
> - ++b;
> - }
> - }
> - OS << '"';
> - }
> + OS << "\t.section\t";
> + printName(OS, getSectionName());
>
> // Handle the weird solaris syntax if desired.
> if (MAI.usesSunStyleELFSectionSwitchSyntax() &&
> @@ -135,8 +139,11 @@ void MCSectionELF::PrintSwitchToSection(
> OS << "," << EntrySize;
> }
>
> - if (Flags & ELF::SHF_GROUP)
> - OS << "," << Group->getName() << ",comdat";
> + if (Flags & ELF::SHF_GROUP) {
> + OS << ",";
> + printName(OS, Group->getName());
> + OS << ",comdat";
> + }
> OS << '\n';
>
> if (Subsection)
>
> Modified: llvm/trunk/lib/Target/MSP430/MCTargetDesc/MSP430MCAsmInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/MSP430/MCTargetDesc/MSP430MCAsmInfo.cpp?rev=194575&r1=194574&r2=194575&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/MSP430/MCTargetDesc/MSP430MCAsmInfo.cpp
> (original)
> +++ llvm/trunk/lib/Target/MSP430/MCTargetDesc/MSP430MCAsmInfo.cpp Wed Nov
> 13 08:01:59 2013
> @@ -24,6 +24,5 @@ MSP430MCAsmInfo::MSP430MCAsmInfo(StringR
> CommentString = ";";
>
> AlignmentIsInBytes = false;
> - AllowNameToStartWithDigit = true;
> UsesELFSectionDirectiveForBSS = true;
> }
>
> Modified: llvm/trunk/lib/Target/Mangler.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mangler.cpp?rev=194575&r1=194574&r2=194575&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/Mangler.cpp (original)
> +++ llvm/trunk/lib/Target/Mangler.cpp Wed Nov 13 08:01:59 2013
> @@ -23,17 +23,6 @@
> #include "llvm/Support/raw_ostream.h"
> using namespace llvm;
>
> -static bool isAcceptableChar(char C, bool AllowPeriod) {
> - if ((C < 'a' || C > 'z') &&
> - (C < 'A' || C > 'Z') &&
> - (C < '0' || C > '9') &&
> - C != '_' && C != '$' && C != '@' &&
> - !(AllowPeriod && C == '.') &&
> - !(C & 0x80))
> - return false;
> - return true;
> -}
> -
> static char HexDigit(int V) {
> return V < 10 ? V+'0' : V+'A'-10;
> }
> @@ -45,46 +34,6 @@ static void MangleLetter(SmallVectorImpl
> OutName.push_back('_');
> }
>
> -/// NameNeedsEscaping - Return true if the identifier \p Str needs quotes
> -/// for this assembler.
> -static bool NameNeedsEscaping(StringRef Str, const MCAsmInfo *MAI) {
> - assert(!Str.empty() && "Cannot create an empty MCSymbol");
> -
> - // If the first character is a number and the target does not allow
> this, we
> - // need quotes.
> - if (!MAI->doesAllowNameToStartWithDigit() && Str[0] >= '0' && Str[0] <=
> '9')
> - return true;
> -
> - // If any of the characters in the string is an unacceptable character,
> force
> - // quotes.
> - bool AllowPeriod = MAI->doesAllowPeriodsInName();
> - for (unsigned i = 0, e = Str.size(); i != e; ++i)
> - if (!isAcceptableChar(Str[i], AllowPeriod))
> - return true;
> - return false;
> -}
> -
> -/// appendMangledName - Add the specified string in mangled form if it
> uses
> -/// any unusual characters.
> -static void appendMangledName(SmallVectorImpl<char> &OutName, StringRef
> Str,
> - const MCAsmInfo *MAI) {
> - // The first character is not allowed to be a number unless the target
> - // explicitly allows it.
> - if (!MAI->doesAllowNameToStartWithDigit() && Str[0] >= '0' && Str[0] <=
> '9') {
> - MangleLetter(OutName, Str[0]);
> - Str = Str.substr(1);
> - }
> -
> - bool AllowPeriod = MAI->doesAllowPeriodsInName();
> - for (unsigned i = 0, e = Str.size(); i != e; ++i) {
> - if (!isAcceptableChar(Str[i], AllowPeriod))
> - MangleLetter(OutName, Str[i]);
> - else
> - OutName.push_back(Str[i]);
> - }
> -}
> -
> -
> /// appendMangledQuotedName - On systems that support quoted symbols, we
> still
> /// have to escape some (obscure) characters like " and \n which would
> break the
> /// assembler's lexing.
> @@ -134,22 +83,14 @@ void Mangler::getNameWithPrefix(SmallVec
> OutName.append(Prefix, Prefix+strlen(Prefix));
> }
> }
> -
> +
> // If this is a simple string that doesn't need escaping, just append
> it.
> - if (!NameNeedsEscaping(Name, MAI) ||
> - // If quotes are supported, they can be used unless the string
> contains
> - // a quote or newline.
> - (MAI->doesAllowQuotesInName() &&
> - Name.find_first_of("\n\"") == StringRef::npos)) {
> + // Quotes can be used unless the string contains a quote or newline.
> + if (Name.find_first_of("\n\"") == StringRef::npos) {
> OutName.append(Name.begin(), Name.end());
> return;
> }
> -
> - // On systems that do not allow quoted names, we need to mangle most
> - // strange characters.
> - if (!MAI->doesAllowQuotesInName())
> - return appendMangledName(OutName, Name, MAI);
> -
> +
> // Okay, the system allows quoted strings. We can quote most anything,
> the
> // only characters that need escaping are " and \n.
> assert(Name.find_first_of("\n\"") != StringRef::npos);
>
> Modified: llvm/trunk/lib/Target/NVPTX/MCTargetDesc/NVPTXMCAsmInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/NVPTX/MCTargetDesc/NVPTXMCAsmInfo.cpp?rev=194575&r1=194574&r2=194575&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/NVPTX/MCTargetDesc/NVPTXMCAsmInfo.cpp (original)
> +++ llvm/trunk/lib/Target/NVPTX/MCTargetDesc/NVPTXMCAsmInfo.cpp Wed Nov 13
> 08:01:59 2013
> @@ -35,8 +35,6 @@ NVPTXMCAsmInfo::NVPTXMCAsmInfo(const Str
>
> PrivateGlobalPrefix = "$L__";
>
> - AllowPeriodsInName = false;
> -
> HasSetDirective = false;
>
> HasSingleParameterDotFile = false;
>
> Modified: llvm/trunk/lib/Target/R600/MCTargetDesc/AMDGPUMCAsmInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/R600/MCTargetDesc/AMDGPUMCAsmInfo.cpp?rev=194575&r1=194574&r2=194575&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/R600/MCTargetDesc/AMDGPUMCAsmInfo.cpp (original)
> +++ llvm/trunk/lib/Target/R600/MCTargetDesc/AMDGPUMCAsmInfo.cpp Wed Nov 13
> 08:01:59 2013
> @@ -31,9 +31,6 @@ AMDGPUMCAsmInfo::AMDGPUMCAsmInfo(StringR
> InlineAsmStart = ";#ASMSTART";
> InlineAsmEnd = ";#ASMEND";
> AssemblerDialect = 0;
> - AllowQuotesInName = false;
> - AllowNameToStartWithDigit = false;
> - AllowPeriodsInName = false;
>
> //===--- Data Emission Directives
> -------------------------------------===//
> ZeroDirective = ".zero";
>
> Modified: llvm/trunk/test/CodeGen/X86/GC/ocaml-gc.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/GC/ocaml-gc.ll?rev=194575&r1=194574&r2=194575&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/GC/ocaml-gc.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/GC/ocaml-gc.ll Wed Nov 13 08:01:59 2013
> @@ -2,23 +2,23 @@
>
> define i32 @main(i32 %x) nounwind gc "ocaml" {
> ; CHECK: .text
> -; CHECK-NEXT: .globl caml_3C_stdin_3E___code_begin
> -; CHECK-NEXT: caml_3C_stdin_3E___code_begin:
> +; CHECK-NEXT: .globl "caml<stdin>__code_begin"
> +; CHECK-NEXT: "caml<stdin>__code_begin":
> ; CHECK-NEXT: .data
> -; CHECK-NEXT: .globl caml_3C_stdin_3E___data_begin
> -; CHECK-NEXT: caml_3C_stdin_3E___data_begin:
> +; CHECK-NEXT: .globl "caml<stdin>__data_begin"
> +; CHECK-NEXT: "caml<stdin>__data_begin":
>
> %puts = tail call i32 @foo(i32 %x)
> ret i32 0
>
> -; CHECK: .globl caml_3C_stdin_3E___code_end
> -; CHECK-NEXT: caml_3C_stdin_3E___code_end:
> +; CHECK: .globl "caml<stdin>__code_end"
> +; CHECK-NEXT: "caml<stdin>__code_end":
> ; CHECK-NEXT: .data
> -; CHECK-NEXT: .globl caml_3C_stdin_3E___data_end
> -; CHECK-NEXT: caml_3C_stdin_3E___data_end:
> +; CHECK-NEXT: .globl "caml<stdin>__data_end"
> +; CHECK-NEXT: "caml<stdin>__data_end":
> ; CHECK-NEXT: .quad 0
> -; CHECK-NEXT: .globl caml_3C_stdin_3E___frametable
> -; CHECK-NEXT: caml_3C_stdin_3E___frametable:
> +; CHECK-NEXT: .globl "caml<stdin>__frametable"
> +; CHECK-NEXT: "caml<stdin>__frametable":
> ; CHECK-NEXT: .short 1
> ; CHECK-NEXT: .align 8
> ; CHECK-NEXT: # live roots for main
>
> Modified: llvm/trunk/test/CodeGen/X86/global-sections.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/global-sections.ll?rev=194575&r1=194574&r2=194575&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/global-sections.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/global-sections.ll Wed Nov 13 08:01:59 2013
> @@ -65,10 +65,10 @@
> ; PR4584
> @"foo bar" = linkonce global i32 42
>
> -; LINUX: .type foo_20_bar, at object
> -; LINUX: .section .data.foo_20_bar,"aGw", at progbits,foo_20_bar,comdat
> -; LINUX: .weak foo_20_bar
> -; LINUX: foo_20_bar:
> +; LINUX: .type "foo bar", at object
> +; LINUX: .section ".data.foo bar","aGw", at progbits,"foo bar",comdat
> +; LINUX: .weak "foo bar"
> +; LINUX: "foo bar":
>
> ; DARWIN: .section __DATA,__datacoal_nt,coalesced
> ; DARWIN: .globl "_foo bar"
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140307/f464c5e3/attachment.html>
More information about the llvm-commits
mailing list