llvm dev - Nov 2009 - [LLVMdev] #define LLVM_ASM_PREFIX_CHAR '\1' to make code more readable?

Hi,

llvm, clang and llvm-gcc contain numerous references to 1, '\1',
'\01', '\001', "\01" and "\01foo". This
makes it difficult for one to
find all occurences of this prefix character and lacks a central place
where the prefix could be documented. (See for example
http://llvm.org/bugs/show_bug.cgi?id=5480 )

How about adding a #define for this prefix? The attached three patches
contain occurences of this prefix in llvm, clang and llvm-gcc that I
was able to identify but I don't know what would be a proper place for
the #define itself. Also should it maybe be LLVM_ASM_PREFIX or just
ASM_PREFIX? (I think both character and string versions are
needed. Afaik characters can not be concatenated to strings with
the preprocessor).

I am also not quite sure how the prefix should be documented. Does
"\01foo" mean "foo is a name that can be given to assembler or
dlsym()
and should not ever be mangled in any way"?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm-asm-prefix1.patch
Type: text/x-diff
Size: 5201 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20091118/f3c17d58/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm-gcc-asm-prefix1.patch
Type: text/x-diff
Size: 2825 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20091118/f3c17d58/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang-asm-prefix1.patch
Type: text/x-diff
Size: 31109 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20091118/f3c17d58/attachment-0002.patch>

On Nov 18, 2009, at 10:17 AM, Timo Juhani Lindfors
wrote:
> Hi,
> 
> llvm, clang and llvm-gcc contain numerous references to 1, '\1',
> '\01', '\001', "\01" and "\01foo". This
makes it difficult for one to
> find all occurences of this prefix character and lacks a central place
> where the prefix could be documented. (See for example
> http://llvm.org/bugs/show_bug.cgi?id=5480 )
> 
> How about adding a #define for this prefix? The attached three patches
> contain occurences of this prefix in llvm, clang and llvm-gcc that I
> was able to identify but I don't know what would be a proper place for
> the #define itself. Also should it maybe be LLVM_ASM_PREFIX or just
> ASM_PREFIX? (I think both character and string versions are
> needed. Afaik characters can not be concatenated to strings with
> the preprocessor).
Changing this from being a magic number to a 'static const char' is fine
with me, please don't use a macro though.  The declaration of this should go
in Mangler.h
> I am also not quite sure how the prefix should be documented. Does
> "\01foo" mean "foo is a name that can be given to assembler
or dlsym()
> and should not ever be mangled in any way"?
\01 means 'do not add the "USER LABEL PREFIX" for the target',
names definitely do need to be mangled for some targets even without this.  For
example, elf systems can't handle symbols with spaces in them.

-Chris

Chris Lattner <clattner at apple.com> writes:
> Changing this from being a magic number to a 'static const char' is
> fine with me, please don't use a macro though.
I do not like macros either. However, how should I remove magic
numbers from

| } else if (Name == "\1stat64" ||
|            Name == "\1lstat64" ||
|            Name == "\1statvfs64" ||
|            Name == "\1__isoc99_sscanf") {

without using a macro? Something like

| } else if (Name[0] == llvm_asm_prefix && Name.slice(1, 0) ==
"stat64" ||
|            Name[0] == llvm_asm_prefix && Name.slice(1, 0) ==
"lstat64" ||
|            Name[0] == llvm_asm_prefix && Name.slice(1, 0) ==
"statvfs64" ||
|            Name[0] == llvm_asm_prefix && Name.slice(1, 0) ==
"__isoc99_sscanf") {

does not look very nice.
>  The declaration of this should go in Mangler.h
Ok.
> \01 means 'do not add the "USER LABEL PREFIX" for the
target', names
> definitely do need to be mangled for some targets even without this.
> For example, elf systems can't handle symbols with spaces in them.
Thanks, with this description I was able to find

/* User label prefix in effect for this compilation.  */
extern const char *user_label_prefix;

from llvm-gcc's output.h. Is llvm_asm_prefix still a good name for the
"\1" prefix?