Re: [PATCH] Add support for the __flash qualifier on AVR

[Pedro Alves <palves@redhat.com>]

On 07/23/2014 05:52 PM, Pierre Langlois wrote:
> Hi Pedro,
> 
> On 21/07/14 16:45, Pedro Alves wrote:
>> Hi Pierre,
>>
>> On 07/08/2014 11:54 AM, Pierre Langlois wrote:
>>> The __flash qualifier is part of the named address spaces for AVR [1]. It allows
>>> putting read-only data in the flash memory, normally reserved for code.
>>>
>>> When used together with a pointer, the DW_AT_address_class attribute is set to 1
>>> and allows GDB to detect that when it will be dereferenced, the data will be
>>> loaded from the flash memory (with the LPM instruction).
>>>
>>
>> Thanks.  This looks good to me, with a couple nits pointed out
>> below addressed.
>>
> 
> Thanks for looking at this.  I have committed a better version of this patch
> last week after review.  So I just submitted another patch improving the
> comments as suggested.
>

Whoops, I completely missed that.  Sorry about that.


> Yes you're right, I mentioned the $pc register because of its type.  I
> understand it's confusing now.

> 
>>
>>>    /* Is it a code address?  */
>>> -  if (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_FUNC
>>> -      || TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_METHOD
>>> -      || TYPE_CODE_SPACE (TYPE_TARGET_TYPE (type)))
>>> +  else if (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_FUNC
>>> +	   || TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_METHOD
>>> +	   || TYPE_CODE_SPACE (TYPE_TARGET_TYPE (type)))
>>
>> Particularly since this bit didn't get that comment?
>>
>> BTW, interesting/curious that TYPE_CODE_SPACE is handled
>> here but not in avr_address_to_pointer.  Offhand, looks like a
>> bug, though off topic for this patch.
>>
> 
> I am bit puzzled here but I suspect this was done on purpose.  I'll submit a
> patch so we can discuss it.
> 

Thanks!

-- 
Thanks,
Pedro Alves