RE: New keywords (string32) for GNU as ?

["Dave Korn" <dave dot korn at artimi dot com>]

On 19 November 2006 14:23, Helge Deller wrote:

> gcc's assembler output for this C-Code:
> 
> 	static wchar_t x[] = L"HELLO";
> 
> is (on little endian targets) something like this:
> x:
>         .string "H"
>         .string ""
>         .string ""
>         .string "E
>         .string ""
>         .string ""
>         .string "L"
> 	.....

  ???  That shows 24-bit chars, AYS ???!  I get

_x:
        .ascii "H\0E\0L\0L\0O\0\0\0"

on x86/cygwin.

> The patch below introduces a new keyword ".string32" to GNU as,
> which creates the same code as the lines above with less commands:
> x:
> 	.string32 "HELLO"

  Don't you want .string16 here?  What platform uses 32-bit wchar_t?  It's 16
everywhere I'm familiar with.

> Would this kind of new keyword "in principle" be acceptable for GNU
> as/binutils ? 

  Disclaimer:  IANA maintainer.

  That said, I think your patch is well motivated and a reasonable solution to
the problem.  I haven't tested it yet but it looks sane.

> --- gas/read.c	29 Aug 2006 15:19:43 -0000	1.121
> +++ gas/read.c	19 Nov 2006 13:59:21 -0000
> @@ -408,6 +408,9 @@ static const pseudo_typeS potable[] = {
>    {"stabn", s_stab, 'n'},
>    {"stabs", s_stab, 's'},
>    {"string", stringer, 1},
> +  {"string8", stringer, 1},
> +  {"string16", stringer, 16+1},
> +  {"string32", stringer, 32+1},
>    {"struct", s_struct, 0},
>  /* tag  */
>    {"text", s_text, 0},

  Once you've cleared up the confusion over what bitsize we're actually
discussing here, you should consider whether string32 is actually needed or
not; then again, it's not like it would do any harm, so you should also
consider whether to add .string64 just for completeness.  You should also
consider offering .ascii[8/16/32] and .asciz[8/16/32] as well, since .asciz is
a synonym for .string.

> @@ -4654,6 +4657,30 @@ s_leb128 (int sign)
>    input_line_pointer--;
>    demand_empty_rest_of_line ();
>  }
> +
> +static void stringer_append_char( int c, int bitsize )
> +{
> +   if (bitsize == 8 || !target_big_endian)
> +	FRAG_APPEND_1_CHAR(c);
> +   switch (bitsize) {
> +	case 8:
> +		return;
> +	case 32:
> +		FRAG_APPEND_1_CHAR(0);
> +		FRAG_APPEND_1_CHAR(0);
> +		/* fall through */
> +	case 16:
> +		FRAG_APPEND_1_CHAR(0);
> +		/* fall through */
> +	default:
> +		if (!target_big_endian)
> +			return;
> +		break;
> +   }
> +   /* on big endian append character */
> +   FRAG_APPEND_1_CHAR(c);
> +}

  If this routine is called with any value other than 8, 16 or 32 (and perhaps
64 if you decide to go that far), it's a serious coding error on the part of
the caller that should be flagged, so I wouldn't as a matter of style accept
default values in that switch: you should recast it like ...


> +   switch (bitsize) {
> +	case 8:
> +		return;
> +	case 32:
> +		FRAG_APPEND_1_CHAR(0);
> +		FRAG_APPEND_1_CHAR(0);
> +		/* fall through */
> +	case 16:
> +		FRAG_APPEND_1_CHAR(0);
> +		if (!target_big_endian)
> +			return;
> +		break;
> +	default:
              /* Called with invalid bitsize argument.  */
              abort ();
> +   }

  You might get pulled up on the coding style of those fall-through comments:

http://www.gnu.org/prep/standards/html_node/Comments.html#Comments
"Please put two spaces after the end of a sentence in your comments, so that
the Emacs sentence commands will work. Also, please write complete sentences
and capitalize the first word."

> +		/* Fall through.  */

would be preferred.

> @@ -4662,9 +4689,10 @@ s_leb128 (int sign)
>  void
>  stringer (/* Worker to do .ascii etc statements.  */
>  	  /* Checks end-of-line.  */
> -	  register int append_zero	/* 0: don't append '\0', else 1.  */)
> +	  register int bitsize_and_append_zero	/* 0: don't append '\0', else
1. 

  The comment should be updated as well as the variable name.  It'll start to
get a bit long when you do that, it'll probably need wrapping.  It might be ok
to remove the 'register' keyword, that's been pretty much an irrelevancy this
century... 

  It all looks good.  You'll need to provide a patch for the docs and
preferably a testcase or two if it's accepted, and of course a ChangeLog
entry.

    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....