Re: [PATCH] optimized out registers in mi

[Pedro Alves <palves@redhat.com>]

On 07/05/2013 05:37 PM, Andrew Burgess wrote:
> On 05/07/2013 3:03 PM, Pedro Alves wrote:
>> On 07/05/2013 02:27 PM, Andrew Burgess wrote:
>>
>>> [1] I applied a patch yesterday that changed value_optimized_out,
>>> before I applied yesterdays patch the behaviour of -data-list-registers
>>> was a bit random, if the register value was lazy then you would get
>>> the "<optimized out>" result, if the value was not lazy then you got
>>> the error.  Of the two behaviours I think returning the
>>> "<optimized out>" string is by far the most helpful so that's
>>> the behaviour we now get in all cases.
>>
>> Agreed.
>>
>> It'd be nice to handle partially optimized out registers
>> too, though I'm not sure gcc does something like that
>> currently.  And that can be always handled later.  This
>> is already a good step.
>>
>>> gdb/ChangeLog
>>
>>> -  if (format == 'N')
>>> +  /* Displaying optimized out values using anything other than the default
>>> +     format will result in the value 0 being shown.  */
>>
>> Please make that s/will/would/.  It's confusing as is; it made
>> me think that printing 0 was what the patch was meaning to do.
>>
>> But, this looks hacky to me.  Do we print 0 with the CLI as well?
>> If not, what's handling it that's missing in MI?  Isn't this being
>> handled gracefully somewhere within val_print itself (I'd
>> think in val_print_scalar_formatted)?
> 
> The answer to this is a few lines down in the function, it is really the
> 'r' formatting case that I'm trying to avoid, as this is handled as a
> special within the output_register function.

Alright, then the comment was wrong.

> 
> So, I could probably write:
> 
>   if ((value_optimized_out (val) && format == 'r') || format == 'N')

That'd already be much better.

> 
> NOTE: The format == 'N' stuff is not mine, and was there before.
> 
> I didn't write this as I figured the result would be the same whatever
> format you send into the print code if the value is optimized_out.
> 
> Now, the next question that comes to me is, why do we handle 'r' as a
> special case inside output_register.... I can't answer that one, maybe
> we could / should move it into the generic print code, then you'd be
> correct and the whole optimized_out check would not be needed.  Would
> your prefer me to try to move the format-'r' code, or is my original
> patch ok?

Well, I'm still correct.  ;-)  It's quite likely/possible that at
some point e.g., the t/Binary format will print the available
bits of partially optimized out registers (e.g., only-half-registers
pushed to stack).  Better not frob the format the frontend requested.
Or the default for vector optimized out values could e.g.,
print half valid vector, half optimized vector, in natural format
(while we want raw).

I can't answer why we special case 'r'.  If we don't push this
down to val_print, I'd prefer:

  if (format == 'r')
    {
	if (value_optimized_out (val))
          {
            stb = mem_fileopen ();
            old_chain = make_cleanup_ui_file_delete (stb);
            val_print_optimized_out (stb);
            ui_out_field_stream (uiout, "value", stb);
            do_cleanups (old_chain);
          }
        else
          {
            // existing R code.
          }
    }
  else
    {
       val_print
    }

Actually, the 'r'/raw printing bits could be factored out
to a separate function and used by default_print_one_register_info
as well (the similar loop prints the register raw).  Which BTW,
doesn't seem to be handling optimized out registers either?
(or rather, I couldn't find where it's handled, just by 2min
looking around.)

-- 
Pedro Alves