Re: GCC bug?

[Ian Molton]

Ah, this becomes (somewhat) clearer now. And then not...

Following it through, it seems that its possible to generate two
slightly differing prologues;

ignoring other registers, and assuming a tmp reg (r0), you get:

move treg, rampd
push treg
move rampd, 0
...
move treg, rampz
push treg

if AVR_HAVE_RAMP{D,Z}

but this:

mov treg, rampz
push treg
mov rampz, 0

if only AVR_HAVE_RAMPZ is set,

with an extra clear of the z register that weasnt there before.

So on AVRs with both RAMPD and RAMPZ, RAMPD is cleared, but RAMPZ is not.

I presume then that the compiler treats RAMPZ specially on those parts,
never assuming it to be zero.

...but then why doesn't that apply on parts with RAMPZ, but without
RAMPD (most AVRs?) I dont see anything in the gcc-avr ABI docs that
states it treats RAMPZ differently on different AVRs...

Its also counter to the comment a few lines above that reads:

/* Push and clear RAMPD/X/Y/Z if present and low-part register is used.
         ??? There are no dwarf2 columns reserved for RAMPD/X/Y/Z.  */

which seems to imply that RAMPZ is to be zeroed unconditionally.

-Ian


On 17/12/2021 22:01, Bruce D. Lightner wrote:
> Ian,
> 
> Trying again... :-)
> 
> No, the "last one" should NOT be AVR_HAVE_RAMPZ.
> 
> The Z register and the RAMPD register are used together to address
> memory in an AVR part which supports RAMPD.
> 
> Some later AVR micrcontrollers have a "Z" register (i.e., AVR_HAVE_RAMPZ
> is set).  Newer, more capable versions of those same AVR parts can ALSO
> have a "RAMPD" register (i.e., AVR_HAVE_RAMPD is set).  If the AVR part
> has a "RAMPD" register then it will ALWAYS have a "Z" register.
> 
> The code you referenced changes the behavior of "emit_push_sfr()"
> depending upon whether the AVR part has both registers or only the Z
> register.
> 
> If there is no Z register then there NEVER is a RAMPD register.  In that
> case neither AVR_HAVE_RAMPZ or AVR_HAVE_RAMPD will be set and
> "emit_push_sfr()" will never be called in the context you referenced.
> 
> Best regards,
> 
> Bruce
> 
> ------------------------------------------------------------------------
> On 12/17/2021 1:33 PM, Ian Molton wrote:
>> Ok, I don't want to sound rude, but did anyone actually *read* what I wrote?
>>
>> I'll highlight the parts of lines in question this time, perhaps that
>> will help...
>>
>> if (AVR_HAVE_RAMPZ
>>     ^^^^^^^^^^^^^^
>>            && TEST_HARD_REG_BIT (set, REG_Z)
>>            && TEST_HARD_REG_BIT (set, REG_Z + 1))
>>          {
>>            emit_push_sfr (rampz_rtx, false /* frame */, AVR_HAVE_RAMPD,
>>                                                         ^^^^^^^^^^^^^^
>>  treg);
>>          }
>>
>> OVER HERE ------------->>>>>--------------------------^^^^^^^^^^^^^^^^^
>>
>>
>> Either I'm missing something here, or someone needs to explain to me why
>> a test for AVR_HAVE_RAMPZ preceeds the use of AVR_HAVE_RAMPD in the code.
>>
>> Shouldn't that last one also be RAMPZ ?
>>
>> If not, why?
>>
>> -Ian
>>
>>
>> On 17/12/2021 16:53, Bruce D. Lightner wrote:
>>> Ian,
>>>
>>> I apologize if I was being obtuse! :-)
>>>
>>> The AVR_HAVE_RAMPD #define tells the routine "emit_push_sfr()" to save
>>> the RAMPD register, but only if the third parameter to the call is
>>> non-zero.  The constant AVR_HAVE_RAMPD will be non-zero if the AVR
>>> micro-architecture supports a RAMPD register.
>>>
>>> That's why you see the code you reported.  Much older versions of
>>> avr-gcc have "emit_push_sfr()" source code with one less parameter
>>> (i.e., no AVR_HAVE_RAMPD reference) because the RAMPD "paging" register
>>> is a relatively "new" AVR feature.
>>>
>>> Best regards,
>>>
>>> Bruce
>>>
>>> ------------------------------------------------------------------------
>>> On 12/17/2021 5:39 AM, Ian Molton wrote:
>>>> I think you misunderstood me;
>>>>
>>>> I know this is where the compiler will save RAMPZ where needed.
>>>>
>>>> So why is it referencing RAMP *D* ?
>>>>
>>>> On 17/12/2021 00:50, Bruce D. Lightner wrote:
>>>>> Ian,
>>>>>
>>>>> Just from a purely AVR architecture point of view, the AVR_HAVE_RAMPD
>>>>> #define indicates that the AVR chip in question supports the "RAMP"
>>>>> paging register, described as follows:
>>>>>
>>>>>    *RAMPD*
>>>>>    Register concatenated with the Z-register enabling direct addressing
>>>>> of the whole data space on MCUs
>>>>>    with more than 64KB data space.
>>>>>
>>>>> Eventually the originally tiny AVR chips' addressable memory got so big
>>>>> that we needed a "paging" register.  (That's a repeating theme with
>>>>> every 8-bit/16-bit microcontroller family.)
>>>>>
>>>>> So the "RAMPD" register needs to be saved along with the X, Y and Z
>>>>> "special function" registers, if it is present.
>>>>>
>>>>> Best regards,
>>>>>
>>>>> Bruce
>>>>>
>>>>> ------------------------------------------------------------------------
>>>>> On 12/16/2021 3:46 PM, Ian Molton wrote:
>>>>>> Browsing the GCC source, I found this in gcc/config/avr/avr.c
>>>>>>
>>>>>>  if (AVR_HAVE_RAMPZ
>>>>>>           && TEST_HARD_REG_BIT (set, REG_Z)
>>>>>>           && TEST_HARD_REG_BIT (set, REG_Z + 1))
>>>>>>         {
>>>>>>           emit_push_sfr (rampz_rtx, false /* frame */, AVR_HAVE_RAMPD,
>>>>>> treg);
>>>>>>         }
>>>>>>
>>>>>>
>>>>>> I wont pretend to fully understand this part of the compiler, but that
>>>>>> AVR_HAVE_RAMPD looks shady to me?
>>>>>>
>>>>>> Anyone with deeper knowledge want to have a look?
>>>>>>
>>>>>> -Ian
>>>>>>
>>>
> 
> 
> -- 
> *Bruce D. Lightner*
> *Lightner Engineering*
> La Jolla, California 92037-3044
> Email: lightner@lightner.net
> URL: http://www.lightner.net/lightner/bruce/