diff mbox

[AVR] : Use EIND consistently

Message ID 4EA14709.7020206@gjlay.de
State New
Headers show

Commit Message

Georg-Johann Lay Oct. 21, 2011, 10:18 a.m. UTC
This patch adds support to consistently use EIND.

The compiler never sets this SFR but uses it in table jumps and EIJMP/EICALL
instructions.

Custom startup code could set EIND to an other value than 0 and the compiler
should use EIND consistently given that EIND might not be zero.

EIND != 0 will need support of custom linker script to locate jump pads in an
other segment, but that's a different story.

The patch undoes some changes from r179760 and introduces using EIND the other
way round: Not trying to avoid EIND altogether and assume code is supposed to
work in the lower segment only, but instead use EIND and not zero-reg when
simulating indirect jump by means of RET.

With this patch, the application may set EIND to a custom value and invent own
linker script to place jump pads.  The assertion is that EIND never changes
throughout the application and therefore ISR prologue/epilogue need not care.

With the gs() magic, code using indirect jumps works fine with that, e.g.
- Indirect calls
- Computed goto
- Jumping to 1: in prologue_saves

What does not work as expected is to jump to const_int addresses like

int main()
{
   ((void(*)(void))0)();
   return 0;
}

Instead, code must read

extern void my_address (void);

int main()
{
    my_address();
    return 0;
}

and compiled with, say -Wl,--defsym,my_address=0x20000, so that a jump pad is
generated.

Patch ok for trunk?

Johann

	* config/avr/libgcc.S (__EIND__): New define to 0x3C.
	(__tablejump__): Consistently use EIND for indirect jump/call.
	(__tablejump_elpm__): Ditto.

Comments

Denis Chertykov Oct. 21, 2011, 10:59 a.m. UTC | #1
2011/10/21 Georg-Johann Lay <avr@gjlay.de>:
> This patch adds support to consistently use EIND.
>
> The compiler never sets this SFR but uses it in table jumps and EIJMP/EICALL
> instructions.
>
> Custom startup code could set EIND to an other value than 0 and the compiler
> should use EIND consistently given that EIND might not be zero.
>
> EIND != 0 will need support of custom linker script to locate jump pads in an
> other segment, but that's a different story.
>
> The patch undoes some changes from r179760 and introduces using EIND the other
> way round: Not trying to avoid EIND altogether and assume code is supposed to
> work in the lower segment only, but instead use EIND and not zero-reg when
> simulating indirect jump by means of RET.
>
> With this patch, the application may set EIND to a custom value and invent own
> linker script to place jump pads.  The assertion is that EIND never changes
> throughout the application and therefore ISR prologue/epilogue need not care.
>
> With the gs() magic, code using indirect jumps works fine with that, e.g.
> - Indirect calls
> - Computed goto
> - Jumping to 1: in prologue_saves
>
> What does not work as expected is to jump to const_int addresses like
>
> int main()
> {
>   ((void(*)(void))0)();
>   return 0;
> }
>
> Instead, code must read
>
> extern void my_address (void);
>
> int main()
> {
>    my_address();
>    return 0;
> }
>
> and compiled with, say -Wl,--defsym,my_address=0x20000, so that a jump pad is
> generated.
>
> Patch ok for trunk?
>
> Johann
>
>        * config/avr/libgcc.S (__EIND__): New define to 0x3C.
>        (__tablejump__): Consistently use EIND for indirect jump/call.
>        (__tablejump_elpm__): Ditto.

Approved.

Denis.
Georg-Johann Lay Oct. 21, 2011, 12:22 p.m. UTC | #2
>> This patch adds support to consistently use EIND.
>>
>> The compiler never sets this SFR but uses it in table jumps and EIJMP/EICALL
>> instructions.
>>
>> Custom startup code could set EIND to an other value than 0 and the compiler
>> should use EIND consistently given that EIND might not be zero.
>>
>> EIND != 0 will need support of custom linker script to locate jump pads in an
>> other segment, but that's a different story.
>>
>> The patch undoes some changes from r179760 and introduces using EIND the other
>> way round: Not trying to avoid EIND altogether and assume code is supposed to
>> work in the lower segment only, but instead use EIND and not zero-reg when
>> simulating indirect jump by means of RET.
>>
>> With this patch, the application may set EIND to a custom value and invent own
>> linker script to place jump pads.  The assertion is that EIND never changes
>> throughout the application and therefore ISR prologue/epilogue need not care.
>>
>> With the gs() magic, code using indirect jumps works fine with that, e.g.
>> - Indirect calls
>> - Computed goto
>> - Jumping to 1: in prologue_saves
>>
>> What does not work as expected is to jump to const_int addresses like
>>
>> int main()
>> {
>>   ((void(*)(void))0)();
>>   return 0;
>> }
>>
>> Instead, code must read
>>
>> extern void my_address (void);
>>
>> int main()
>> {
>>    my_address();
>>    return 0;
>> }
>>
>> and compiled with, say -Wl,--defsym,my_address=0x20000, so that a jump pad is
>> generated.
>>
>> Patch ok for trunk?
>>
>> Johann
>>
>>        * config/avr/libgcc.S (__EIND__): New define to 0x3C.
>>        (__tablejump__): Consistently use EIND for indirect jump/call.
>>        (__tablejump_elpm__): Ditto.
> 
> Approved.
> 
> Denis.

Is this a thing to back port?

Johann
Denis Chertykov Oct. 21, 2011, 1:29 p.m. UTC | #3
2011/10/21 Georg-Johann Lay <avr@gjlay.de>:
>>> This patch adds support to consistently use EIND.
>>>
>>> The compiler never sets this SFR but uses it in table jumps and EIJMP/EICALL
>>> instructions.
>>>
>>> Custom startup code could set EIND to an other value than 0 and the compiler
>>> should use EIND consistently given that EIND might not be zero.
>>>
>>> EIND != 0 will need support of custom linker script to locate jump pads in an
>>> other segment, but that's a different story.
>>>
>>> The patch undoes some changes from r179760 and introduces using EIND the other
>>> way round: Not trying to avoid EIND altogether and assume code is supposed to
>>> work in the lower segment only, but instead use EIND and not zero-reg when
>>> simulating indirect jump by means of RET.
>>>
>>> With this patch, the application may set EIND to a custom value and invent own
>>> linker script to place jump pads.  The assertion is that EIND never changes
>>> throughout the application and therefore ISR prologue/epilogue need not care.
>>>
>>> With the gs() magic, code using indirect jumps works fine with that, e.g.
>>> - Indirect calls
>>> - Computed goto
>>> - Jumping to 1: in prologue_saves
>>>
>>> What does not work as expected is to jump to const_int addresses like
>>>
>>> int main()
>>> {
>>>   ((void(*)(void))0)();
>>>   return 0;
>>> }
>>>
>>> Instead, code must read
>>>
>>> extern void my_address (void);
>>>
>>> int main()
>>> {
>>>    my_address();
>>>    return 0;
>>> }
>>>
>>> and compiled with, say -Wl,--defsym,my_address=0x20000, so that a jump pad is
>>> generated.
>>>
>>> Patch ok for trunk?
>>>
>>> Johann
>>>
>>>        * config/avr/libgcc.S (__EIND__): New define to 0x3C.
>>>        (__tablejump__): Consistently use EIND for indirect jump/call.
>>>        (__tablejump_elpm__): Ditto.
>>
>> Approved.
>>
>> Denis.
>
> Is this a thing to back port?
>

As you please.

Denis.
Georg-Johann Lay Oct. 21, 2011, 2:29 p.m. UTC | #4
>>>> This patch adds support to consistently use EIND.
>>>>
>>>> The compiler never sets this SFR but uses it in table jumps and EIJMP/EICALL
>>>> instructions.
>>>>
>>>> Custom startup code could set EIND to an other value than 0 and the compiler
>>>> should use EIND consistently given that EIND might not be zero.
>>>>
>>>> EIND != 0 will need support of custom linker script to locate jump pads in an
>>>> other segment, but that's a different story.
>>>>
>>>> The patch undoes some changes from r179760 and introduces using EIND the other
>>>> way round: Not trying to avoid EIND altogether and assume code is supposed to
>>>> work in the lower segment only, but instead use EIND and not zero-reg when
>>>> simulating indirect jump by means of RET.
>>>>
>>>> With this patch, the application may set EIND to a custom value and invent own
>>>> linker script to place jump pads.  The assertion is that EIND never changes
>>>> throughout the application and therefore ISR prologue/epilogue need not care.
>>>>
>>>> With the gs() magic, code using indirect jumps works fine with that, e.g.
>>>> - Indirect calls
>>>> - Computed goto
>>>> - Jumping to 1: in prologue_saves
>>>>
>>>> What does not work as expected is to jump to const_int addresses like
>>>>
>>>> int main()
>>>> {
>>>>   ((void(*)(void))0)();
>>>>   return 0;
>>>> }
>>>>
>>>> Instead, code must read
>>>>
>>>> extern void my_address (void);
>>>>
>>>> int main()
>>>> {
>>>>    my_address();
>>>>    return 0;
>>>> }
>>>>
>>>> and compiled with, say -Wl,--defsym,my_address=0x20000, so that a jump pad is
>>>> generated.
>>>>
>>>> Patch ok for trunk?
>>>>
>>>> Johann
>>>>
>>>>        * config/avr/libgcc.S (__EIND__): New define to 0x3C.
>>>>        (__tablejump__): Consistently use EIND for indirect jump/call.
>>>>        (__tablejump_elpm__): Ditto.
>>> Approved.
>>>
>>> Denis.
>> Is this a thing to back port?
> 
> As you please.
> 
> Denis.

It's here:

http://gcc.gnu.org/viewcvs?view=revision&revision=180303

Opened PR50820 for this.

Johann
diff mbox

Patch

Index: config/avr/libgcc.S
===================================================================
--- config/avr/libgcc.S	(revision 180262)
+++ config/avr/libgcc.S	(working copy)
@@ -28,6 +28,7 @@  see the files COPYING3 and COPYING.RUNTI
 #define __SP_H__ 0x3e
 #define __SP_L__ 0x3d
 #define __RAMPZ__ 0x3B
+#define __EIND__  0x3C

 /* Most of the functions here are called directly from avr.md
    patterns, instead of using the standard libcall mechanisms.
@@ -821,17 +822,12 @@  ENDF __tablejump2__

 DEFUN __tablejump__
 #if defined (__AVR_HAVE_LPMX__)
-#if defined (__AVR_HAVE_EIJMP_EICALL__)
-	lpm  __tmp_reg__, Z+		
-	push __tmp_reg__
-	lpm  __tmp_reg__, Z		
-	push __tmp_reg__
-	push __zero_reg__
-	ret
-#else
 	lpm __tmp_reg__, Z+
 	lpm r31, Z
 	mov r30, __tmp_reg__
+#if defined (__AVR_HAVE_EIJMP_EICALL__)
+	eijmp
+#else
 	ijmp
 #endif

@@ -842,7 +838,8 @@  DEFUN __tablejump__
 	lpm
 	push r0
 #if defined (__AVR_HAVE_EIJMP_EICALL__)
-	push __zero_reg__
+	in   __tmp_reg__, __EIND__
+	push __tmp_reg__
 #endif
 	ret
 #endif /* !HAVE_LPMX */
@@ -1034,7 +1031,8 @@  DEFUN __tablejump_elpm__
 	elpm
 	push	r0
 #if defined (__AVR_HAVE_EIJMP_EICALL__)
-        push    __zero_reg__
+	in      __tmp_reg__, __EIND__
+	push    __tmp_reg__
 #endif
 	ret
 #endif