[PATCH] New ia64 @slotcount pseudo func

[Douglas B Rupp]

Submitted for comment as per previous discussions.

--Douglas Rupp
AdaCore



        * gas/config/tc-ia64.c (PSEUDO_FUNC_EXPR): New pseudo_func type.
        (@slotcount): New pseudo func of type EXPR.
        (ia64_parse_name): Implement @slotcount.

--- gas/config/tc-ia64.c 2009-10-06 22:13:53.000000000 -0700
+++ gas/config/tc-ia64.c 2009-10-28 11:31:41.300435760 -0800
@@ -545,7 +545,8 @@ static struct
  PSEUDO_FUNC_RELOC,
  PSEUDO_FUNC_CONST,
  PSEUDO_FUNC_REG,
- PSEUDO_FUNC_FLOAT
+ PSEUDO_FUNC_FLOAT,
+ PSEUDO_FUNC_EXPR
       }
     type;
     union
@@ -576,6 +577,9 @@ pseudo_func[] =
     { NULL, 0, { 0 } }, /* placeholder for FUNC_LT_TP_RELATIVE */
     { "iplt", PSEUDO_FUNC_RELOC, { 0 } },
 
+    /* expression pseudo functions:  */
+    { "slotcount", PSEUDO_FUNC_EXPR, { 0 } },
+
     /* mbtype4 constants:  */
     { "alt", PSEUDO_FUNC_CONST, { 0xa } },
     { "brcst", PSEUDO_FUNC_CONST, { 0x0 } },
@@ -7779,6 +7783,72 @@ ia64_parse_name (char *name, expressionS
   *nextcharP = *input_line_pointer;
   break;
 
+ case PSEUDO_FUNC_EXPR:
+  end = input_line_pointer;
+  if (*nextcharP != '(')
+    {
+      as_bad (_("Expected '('"));
+      break;
+    }
+  /* Skip '('.  */
+  ++input_line_pointer;
+
+  /* The only expression pseudo func at this time is @slotcount
+     which takes an expression guaranteed to look like (beg-end),
+     there beg and end are addresses, and figure out how many
+     Itanium instruction slots separate the addresses.  The value
+     is needed for a couple of VMS debugger attributes relating to
+     prologue and epilogue size.  */
+
+  if (strcmp (pseudo_func[idx].name, "slotcount") == 0)
+    {
+      char *name;
+      char c;
+      symbolS *symbolPend, *symbolPbeg;
+      int end, beg, val;
+
+      name = input_line_pointer;
+      c = get_symbol_end ();
+      symbolPend = symbol_find_or_make (name);
+
+      ++input_line_pointer;
+      name = input_line_pointer;
+      c = get_symbol_end ();
+      symbolPbeg = symbol_find_or_make (name);
+
+      /* Calculate the number of instruction slots between the two
+ labels.  They are guaranteed to be in the same (.text)
+ section.  */
+      end = S_GET_VALUE (symbolPend);
+      beg = S_GET_VALUE (symbolPbeg);
+
+      val = (((end & -16) - (beg & -16)) / 16 * 3)
+    + (end & 15)
+    - (beg & 15);
+
+      e->X_op = O_constant;
+      e->X_add_number = val;
+
+      /* Put the ')' back for later error checking.  */
+      *input_line_pointer = ')';
+    }
+  else
+            {
+      /* Do something by default which is not unexpected.  */
+      expression (e);
+    }
+
+  if (*input_line_pointer != ')')
+    {
+      as_bad (_("Missing ')'"));
+      goto done1;
+    }
+  /* Skip ')'.  */
+  ++input_line_pointer;
+ done1:
+  *nextcharP = *input_line_pointer;
+  break;
+
  case PSEUDO_FUNC_CONST:
   e->X_op = O_constant;
   e->X_add_number = pseudo_func[idx].u.ival;

[Alan Modra]

On Wed, Oct 28, 2009 at 01:24:08PM -0700, Douglas B Rupp wrote:
> + case PSEUDO_FUNC_EXPR:
> +  end = input_line_pointer;

This seems an unnecessary assignment to "end".  Not your bug since
you're copying existing code, but no need to make things worse.

> +  if (*nextcharP != '(')
> +    {
> +      as_bad (_("Expected '('"));
> +      break;
> +    }
> +  /* Skip '('.  */
> +  ++input_line_pointer;

*input_line_pointer++ = '('; otherwise we're left with a NUL in the
input line which may result in unusual error messages.

> +
> +  /* The only expression pseudo func at this time is @slotcount
> +     which takes an expression guaranteed to look like (beg-end),
> +     there beg and end are addresses, and figure out how many
> +     Itanium instruction slots separate the addresses.  The value
> +     is needed for a couple of VMS debugger attributes relating to
> +     prologue and epilogue size.  */
> +
> +  if (strcmp (pseudo_func[idx].name, "slotcount") == 0)

Why strcmp if you know that it must be "slotcount"?

> +    {
> +      char *name;
> +      char c;
> +      symbolS *symbolPend, *symbolPbeg;
> +      int end, beg, val;
> +
> +      name = input_line_pointer;
> +      c = get_symbol_end ();

Check for comma here?

> +      symbolPend = symbol_find_or_make (name);
> +
> +      ++input_line_pointer;

*input_line_pointer++ = c;

> +      name = input_line_pointer;
> +      c = get_symbol_end ();
> +      symbolPbeg = symbol_find_or_make (name);
> +
> +      /* Calculate the number of instruction slots between the two
> + labels.  They are guaranteed to be in the same (.text)
> + section.  */
> +      end = S_GET_VALUE (symbolPend);
> +      beg = S_GET_VALUE (symbolPbeg);
> +
> +      val = (((end & -16) - (beg & -16)) / 16 * 3)
> +    + (end & 15)
> +    - (beg & 15);
> +
> +      e->X_op = O_constant;
> +      e->X_add_number = val;
> +
> +      /* Put the ')' back for later error checking.  */
> +      *input_line_pointer = ')';

You should be writing back "c".  Best done immediately after you're
finished with "name".

> +    }
> +  else
> +            {
> +      /* Do something by default which is not unexpected.  */
> +      expression (e);
> +    }
> +
> +  if (*input_line_pointer != ')')
> +    {
> +      as_bad (_("Missing ')'"));
> +      goto done1;
> +    }
> +  /* Skip ')'.  */
> +  ++input_line_pointer;
> + done1:

I'm sure you can rewrite this without the goto.

> +  *nextcharP = *input_line_pointer;
> +  break;
> +
>   case PSEUDO_FUNC_CONST:
>    e->X_op = O_constant;
>    e->X_add_number = pseudo_func[idx].u.ival;


--
Alan Modra
Australia Development Lab, IBM

[Douglas B Rupp]

I tried to address all your concerns, but I didn't understand the
comment about checking for a comma.

Second try submitted for review

--Douglas Rupp
AdaCore



2009-10-29  Douglas B Rupp  <rupp@...>

        * gas/config/tc-ia64.c (PSEUDO_FUNC_EXPR): New pseudo_func type.
        (@slotcount): New pseudo func of type EXPR.
        (ia64_parse_name): Implement @slotcount.

--- gas/config/tc-ia64.c 2009-10-06 22:13:53.000000000 -0700
+++ gas/config/tc-ia64.c 2009-10-28 22:35:06.244566607 -0800
@@ -545,7 +545,8 @@ static struct
  PSEUDO_FUNC_RELOC,
  PSEUDO_FUNC_CONST,
  PSEUDO_FUNC_REG,
- PSEUDO_FUNC_FLOAT
+ PSEUDO_FUNC_FLOAT,
+ PSEUDO_FUNC_EXPR
       }
     type;
     union
@@ -576,6 +577,9 @@ pseudo_func[] =
     { NULL, 0, { 0 } }, /* placeholder for FUNC_LT_TP_RELATIVE */
     { "iplt", PSEUDO_FUNC_RELOC, { 0 } },
 
+    /* expression pseudo functions:  */
+    { "slotcount", PSEUDO_FUNC_EXPR, { 0 } },
+
     /* mbtype4 constants:  */
     { "alt", PSEUDO_FUNC_CONST, { 0xa } },
     { "brcst", PSEUDO_FUNC_CONST, { 0x0 } },
@@ -7779,6 +7783,58 @@ ia64_parse_name (char *name, expressionS
   *nextcharP = *input_line_pointer;
   break;
 
+ case PSEUDO_FUNC_EXPR:
+  if (*nextcharP != '(')
+    {
+      as_bad (_("Expected '('"));
+      break;
+    }
+  /* Skip '('.  */
+  *input_line_pointer++ = '(';
+
+  /* The only expression pseudo func at this time is @slotcount
+     which takes an expression guaranteed to look like (beg-end),
+     there beg and end are addresses, and figure out how many
+     Itanium instruction slots separate the addresses.  The value
+     is needed for a couple of VMS debugger attributes relating to
+     prologue and epilogue size.  */
+
+  {
+    char *name;
+    char c;
+    symbolS *symbolPend, *symbolPbeg;
+    int end, beg, val;
+
+    name = input_line_pointer;
+    c = get_symbol_end ();
+    symbolPend = symbol_find_or_make (name);
+    *input_line_pointer++ = c;
+
+    name = input_line_pointer;
+    c = get_symbol_end ();
+    symbolPbeg = symbol_find_or_make (name);
+    *input_line_pointer = c;
+
+    /* Calculate the number of instruction slots between the two
+       labels.  They are guaranteed to be in the same (.text)
+       section.  */
+    end = S_GET_VALUE (symbolPend);
+    beg = S_GET_VALUE (symbolPbeg);
+
+    val = (((end & -16) - (beg & -16)) / 16 * 3)
+  + (end & 15)
+  - (beg & 15);
+
+    e->X_op = O_constant;
+    e->X_add_number = val;
+  }
+
+  if (*input_line_pointer != ')')
+    as_bad (_("Missing ')'"));
+
+  *nextcharP = *++input_line_pointer;
+  break;
+
  case PSEUDO_FUNC_CONST:
   e->X_op = O_constant;
   e->X_add_number = pseudo_func[idx].u.ival;

[Jan Beulich]

>>> Douglas B Rupp <rupp@...> 28.10.09 21:24 >>>
>Submitted for comment as per previous discussions.

Why can't the debugger do the math that you do here? Unless Intel's
assembler is intending to also support @slotcount, I'm not certain it's a
good idea to add such to gas.

Also:

>+      /* Calculate the number of instruction slots between the two
>+ labels.  They are guaranteed to be in the same (.text)
>+ section.  */
>+      end = S_GET_VALUE (symbolPend);
>+      beg = S_GET_VALUE (symbolPbeg);

Where is this "guarantee" being enforced? I think you mean "They are
supposed...", and need to check this really is the case.

Jan

[Douglas B Rupp]

Jan Beulich wrote:
>>>> Douglas B Rupp <rupp@...> 28.10.09 21:24 >>>
>> Submitted for comment as per previous discussions.
>
> Why can't the debugger do the math that you do here? Unless Intel's
> assembler is intending to also support @slotcount, I'm not certain it's a
> good idea to add such to gas.

This is for the VMS Debugger on IA64. VMS compilers generate object code
directly and don't use the Intel assembler.  The debugger requires a
slotcount here.  There's some history on the gcc list if your interested.

>
> Also:
>
>> +      /* Calculate the number of instruction slots between the two
>> + labels.  They are guaranteed to be in the same (.text)
>> + section.  */
>> +      end = S_GET_VALUE (symbolPend);
>> +      beg = S_GET_VALUE (symbolPbeg);
>
> Where is this "guarantee" being enforced? I think you mean "They are
> supposed...", and need to check this really is the case.

It's only used for function prologue, epilogue offsets which are always
going to be in the same .text section, right?  If you foresee using this
pseudo func for something else, then it needs a little more error
checking or I can add some error checking jic.

>
> Jan
>

[Jan Beulich]

>>> Douglas B Rupp <rupp@...> 29.10.09 09:31 >>>
>Jan Beulich wrote:
>>>>> Douglas B Rupp <rupp@...> 28.10.09 21:24 >>>
>>> Submitted for comment as per previous discussions.
>>
>> Why can't the debugger do the math that you do here? Unless Intel's
>> assembler is intending to also support @slotcount, I'm not certain it's a
>> good idea to add such to gas.
>
>This is for the VMS Debugger on IA64. VMS compilers generate object code
>directly and don't use the Intel assembler.  The debugger requires a
>slotcount here.  There's some history on the gcc list if your interested.

To me this at a first glance looks like the debugger would need fixing
instead of the assembler...

>>
>> Also:
>>
>>> +      /* Calculate the number of instruction slots between the two
>>> + labels.  They are guaranteed to be in the same (.text)
>>> + section.  */
>>> +      end = S_GET_VALUE (symbolPend);
>>> +      beg = S_GET_VALUE (symbolPbeg);
>>
>> Where is this "guarantee" being enforced? I think you mean "They are
>> supposed...", and need to check this really is the case.
>
>It's only used for function prologue, epilogue offsets which are always
>going to be in the same .text section, right?  If you foresee using this
>pseudo func for something else, then it needs a little more error
>checking or I can add some error checking jic.

You intend to use it only that way, which doesn't mean others couldn't
start using it for other things. Unless its use in other than the intended
context gets reported as an error, imo it should make no assumptions
about where it got used.

Btw., couldn't (if you really can't fix the debugger) the addition be made
VMS-specific (so that on platforms where the Intel assembler can be
considered the reference assembler no incompatible extensions become
available)? Alternatively, could you get Intel's buyoff on reserving the
"slotcount" pseudo function name as an optional extension with precisely
the semantics you need?

Jan

[Nick Clifton]

Hi Douglas,

   The patch looks good, but I would suggest that you augment your
parsing of the parameters to the @slotcount pseudo func so that it can
cope with whitespace between the parameters and also handle the
situation where a malformed expression is provided.

Cheers
   Nick

[Alan Modra]

On Thu, Oct 29, 2009 at 12:08:20AM -0700, Douglas B Rupp wrote:
> I tried to address all your concerns, but I didn't understand the  
> comment about checking for a comma.

You have a separator of some sort between the symbols.  Hmm, from
previous posts I see you expect an expression of the form end-beg, and
in fact if I'd read the comment I'd have seen that too.  :-)
Actually @slotinfo(end-beg) strikes me as a little odd.  You aren't
calculating a function of a single variable, so I think it would parse
better as @slotinfo(end,beg).

> +    end = S_GET_VALUE (symbolPend);
> +    beg = S_GET_VALUE (symbolPbeg);

I missed noticing this first time around, but there are some missing
checks here.  Presumably you will only support references to symbols
that are defined, so you should use symbol_find rather than
symbol_find_or_make, and add the following check.

if (symbolPend == NULL
    || symbolPbeg == NULL
    || !S_IS_DEFINED (symbolPend)
    || !S_IS_DEFINED (symbolPbeg)
    || S_GET_SEGMENT (symbolPend) != S_GET_SEGMENT (symbolPbeg)
    || !frag_offset_fixed_p (symbol_get_frag (symbolPend),
                             symbol_get_frag (symbolPbeg),
                             &frag_off))
  as_bad (some_error_message);

You'll also need to adjust your "end" value by frag_off.  At this
stage of assembly, symbol values are offsets within their frags.  The
two symbols may be in different frags.  I hope ia64 gas always creates
insn frags that are multiples of 16 in size.  If it doesn't then you
can't resolve the expression..

--
Alan Modra
Australia Development Lab, IBM

[Douglas B Rupp]

Question: When I look at what gets passed to ia64_parse_name in gdb vs.
what's in the .s file, it appears that extraneous whitespace has already
been removed. Under what circumstances would there be whitespace between
parameters?

Nick Clifton wrote:

> Hi Douglas,
>
>   The patch looks good, but I would suggest that you augment your
> parsing of the parameters to the @slotcount pseudo func so that it can
> cope with whitespace between the parameters and also handle the
> situation where a malformed expression is provided.
>
> Cheers
>   Nick
>

[Alan Modra]

On Wed, Nov 04, 2009 at 12:09:18AM -0800, Douglas B Rupp wrote:
> Question: When I look at what gets passed to ia64_parse_name in gdb vs.  
> what's in the .s file, it appears that extraneous whitespace has already  
> been removed. Under what circumstances would there be whitespace between  
> parameters?

On ia64 you won't need to worry about skipping whitespace since it
will have been removed by the preprocessor.  (On some other targets
you do need to handle whitespace in expressions, due to chars put
in tc_symbol_chars.)

--
Alan Modra
Australia Development Lab, IBM