[llvm] r194575 - Remove AllowQuotesInName and friends from MCAsmInfo.
Kevin Modzelewski
kmod at dropbox.com
Wed Nov 13 18:43:12 PST 2013
Hi, this commit seems to cause some infrequent issue with mcjit. I'm not
100% sure what the exact problem is or how it is related to this patch (I
bisected to find this), but the result is that sometimes the jit'd code
memory comes up with the wrong permissions. Even though it's
deterministic, it's infrequent so unfortunately I don't have a good test
case at the moment; I can work on reducing it but I thought I'd flag the
issue in the meantime. It's also definitely possible that I'm using mcjit
is some brittle way and that this patch just exposes it, but I don't think
that that would cause the type of issue I'm seeing.
Kevin
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
>
> 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/20131113/3c0d8ed8/attachment.html>
More information about the llvm-commits
mailing list