llvm dev - Sep 2009 - [LLVMdev] Patch for PR4776

Hello,

This patch against svn r82147 "fixes" PR4776.  Certain targets (e.g.,
MSP430) allow a symbol to start with a number, e.g. "0x0021"; you can
say stuff like "mov.b &0x0021, r15" to copy a byte from memory
address
0x0021 to register r15.  LLVM had been generating this ill-formed code
instead:

  mov.b   &_30_x0021, r15     # note ASCII '0' has character code
0x30

Chris suggested that I add a bool for "symbols can start with digit"
to MCAsmInfo and set it to true in the MSP430 backend, then make the
mangler and MCSymbol printer respect that bool -- so that's what I
did.

Chris blessed the patch in the Bugzilla thread, but I wanted to
solicit comments here because it's my first.  (Is my approach
reasonable?  Would a more conservative strategy, e.g., just allowing
symbols to start with "0x", suffice?)

My own comments: there's a lot of code duplication between MCSymbol
and Mangler; I'm not sure whether that's intentional or unavoidable.
I also noticed words like "name" and "symbol" being used
interchangeably and somewhat inconsistently; my patch simply follows
the naming schemes apparent in each file I touched.

-ben
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr4776-vs-r82147.patch
Type: application/octet-stream
Size: 5939 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20090917/b29c66ce/attachment.obj>

Looks great to me, please commit.

On Sep 17, 2009, at 10:08 AM, Ben Ransford wrote:
> Hello,
>
> This patch against svn r82147 "fixes" PR4776.  Certain targets
(e.g.,
> MSP430) allow a symbol to start with a number, e.g. "0x0021"; you
can
> say stuff like "mov.b &0x0021, r15" to copy a byte from
memory address
> 0x0021 to register r15.  LLVM had been generating this ill-formed code
> instead:
>
> mov.b   &_30_x0021, r15     # note ASCII '0' has character code
0x30
>
> Chris suggested that I add a bool for "symbols can start with
digit"
> to MCAsmInfo and set it to true in the MSP430 backend, then make the
> mangler and MCSymbol printer respect that bool -- so that's what I
> did.
>
> Chris blessed the patch in the Bugzilla thread, but I wanted to
> solicit comments here because it's my first.  (Is my approach
> reasonable?  Would a more conservative strategy, e.g., just allowing
> symbols to start with "0x", suffice?)
>
> My own comments: there's a lot of code duplication between MCSymbol
> and Mangler; I'm not sure whether that's intentional or
unavoidable.
> I also noticed words like "name" and "symbol" being
used
> interchangeably and somewhat inconsistently; my patch simply follows
> the naming schemes apparent in each file I touched.
>
> -ben
> <pr4776-vs- 
> r82147.patch>_______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev