Re: [PATCH, AVR] Fix PR target/50925, use hard_frame_pointer_rtx
2012/1/14 Denis Chertykov cherty...@gmail.com:
2012/1/13 Georg-Johann Lay a...@gjlay.de:
Denis Chertykov wrote:
Committed
Denis
Consider code prom PR51374
void __vector_18 (void)
{
extern char slot;
unsigned char status = (*(volatile unsigned char*) 0x2B);
unsigned char data = (*(volatile unsigned char*) 0x2C);
if (status 0x10)
slot = 0;
}
the code with -Os -S -dp sets up a frame pointer which is not needed and
should
not be there:
__vector_18:
push r28 ; 28 pushqi1/1 [length = 1]
push r29 ; 29 pushqi1/1 [length = 1]
in r28,__SP_L__ ; 30 *movhi/8 [length = 2]
in r29,__SP_H__
/* prologue: function */
/* frame size = 0 */
/* stack size = 2 */
.L__stack_usage = 2
in r24,0xc ; 8 movqi_insn/4 [length = 1]
sbis 0xb,4 ; 11 *sbix_branch [length = 2]
rjmp .L1
sts slot,__zero_reg__ ; 13 movqi_insn/3 [length = 2]
.L1:
/* epilogue start */
pop r29 ; 33 popqi [length = 1]
pop r28 ; 34 popqi [length = 1]
ret ; 35 return_from_epilogue [length = 1]
This happens even for empty function.
Oops.
Something wrong happened, probably I committed a wrong untested patch.
It must be reverted.
Commit reverted.
Denis.
Re: [PATCH, AVR] Fix PR target/50925, use hard_frame_pointer_rtx
Denis Chertykov wrote: Georg-Johann Lay: Denis Chertykov schrieb: 2) Can we remove from avr.c:avr_option_override() the following: if (avr_strict_X) flag_caller_saves = 0; that hacked around similar spill fails? 3) As PR50775 is fixed: Would it make sense to turn on -mstrict-X per default now, i.e. no more fake X addressing except requested per -mno-strict-X? This bug (and it's fix) isn't related to this addressing problems. The addressing is/was connected to spill fails: -mstrict-X increased register pressure so that there were spill fails if -fcaller-saves was turned on, too. So the question is: will -mstrict-X work together with -fcaller-saves without raising spill fails in difficult reload situations? If -mstrict-X is safe in the way that it does not lead to spill fails because reload cannot cope with the few address registers, then we could turn on -mstrict-X by default and get rid of fake addressing. The reason why -mstrict-X is not on per default was that the risk of spill fails was estimated as too high (no problem of mstrict-X but of reload). Johann
Re: [PATCH, AVR] Fix PR target/50925, use hard_frame_pointer_rtx
2012/1/13 Georg-Johann Lay a...@gjlay.de: Denis Chertykov wrote: Georg-Johann Lay: Denis Chertykov schrieb: 2) Can we remove from avr.c:avr_option_override() the following: if (avr_strict_X) flag_caller_saves = 0; that hacked around similar spill fails? 3) As PR50775 is fixed: Would it make sense to turn on -mstrict-X per default now, i.e. no more fake X addressing except requested per -mno-strict-X? This bug (and it's fix) isn't related to this addressing problems. The addressing is/was connected to spill fails: -mstrict-X increased register pressure so that there were spill fails if -fcaller-saves was turned on, too. So the question is: will -mstrict-X work together with -fcaller-saves without raising spill fails in difficult reload situations? Nothing changed in this area. -mstrict-X will rise spill fails in difficult reload situations. If -mstrict-X is safe in the way that it does not lead to spill fails because reload cannot cope with the few address registers, then we could turn on -mstrict-X by default and get rid of fake addressing. Nothing changed in this area. The reload is a reload as is. The reason why -mstrict-X is not on per default was that the risk of spill fails was estimated as too high (no problem of mstrict-X but of reload). Yes. I'm agree with you - no problem of mstrict-X but of reload. The fake addressing exists only because of this. Denis.
Re: [PATCH, AVR] Fix PR target/50925, use hard_frame_pointer_rtx
Denis Chertykov wrote:
Committed
Denis
Consider code prom PR51374
void __vector_18 (void)
{
extern char slot;
unsigned char status = (*(volatile unsigned char*) 0x2B);
unsigned char data = (*(volatile unsigned char*) 0x2C);
if (status 0x10)
slot = 0;
}
the code with -Os -S -dp sets up a frame pointer which is not needed and should
not be there:
__vector_18:
push r28 ; 28 pushqi1/1 [length = 1]
push r29 ; 29 pushqi1/1 [length = 1]
in r28,__SP_L__ ; 30 *movhi/8[length = 2]
in r29,__SP_H__
/* prologue: function */
/* frame size = 0 */
/* stack size = 2 */
.L__stack_usage = 2
in r24,0xc ; 8 movqi_insn/4[length = 1]
sbis 0xb,4 ; 11 *sbix_branch[length = 2]
rjmp .L1
sts slot,__zero_reg__; 13 movqi_insn/3[length = 2]
.L1:
/* epilogue start */
pop r29 ; 33 popqi [length = 1]
pop r28 ; 34 popqi [length = 1]
ret ; 35 return_from_epilogue[length = 1]
This happens even for empty function.
Johann
Re: [PATCH, AVR] Fix PR target/50925, use hard_frame_pointer_rtx
2012/1/13 Georg-Johann Lay a...@gjlay.de:
Denis Chertykov wrote:
Committed
Denis
Consider code prom PR51374
void __vector_18 (void)
{
extern char slot;
unsigned char status = (*(volatile unsigned char*) 0x2B);
unsigned char data = (*(volatile unsigned char*) 0x2C);
if (status 0x10)
slot = 0;
}
the code with -Os -S -dp sets up a frame pointer which is not needed and
should
not be there:
__vector_18:
push r28 ; 28 pushqi1/1 [length = 1]
push r29 ; 29 pushqi1/1 [length = 1]
in r28,__SP_L__ ; 30 *movhi/8 [length = 2]
in r29,__SP_H__
/* prologue: function */
/* frame size = 0 */
/* stack size = 2 */
.L__stack_usage = 2
in r24,0xc ; 8 movqi_insn/4 [length = 1]
sbis 0xb,4 ; 11 *sbix_branch [length = 2]
rjmp .L1
sts slot,__zero_reg__ ; 13 movqi_insn/3 [length = 2]
.L1:
/* epilogue start */
pop r29 ; 33 popqi [length = 1]
pop r28 ; 34 popqi [length = 1]
ret ; 35 return_from_epilogue [length = 1]
This happens even for empty function.
Oops.
Something wrong happened, probably I committed a wrong untested patch.
It must be reverted.
Denis.
Re: [PATCH, AVR] Fix PR target/50925, use hard_frame_pointer_rtx
2012/1/9 Denis Chertykov cherty...@gmail.com: Hi Georg. I have found that conversion AVR port to using hard_frame_pointer have resolved PR 50925 . I have tested the patch without regressions, but I'm worry about it. Can you test it with your testsuite for regressions ? May be you have your own special difficult tests (special for addressing) ? 2012-01-09 Richard Henderson r...@redhat.com Denis Chertykov cherty...@gmail.com PR target/50925 * config/avr/avr-protos.h (avr_hard_regno_nregs): Declare. * config/avr/avr.c (avr_can_eliminate): Simplify. (avr_initial_elimination_offset): Likewise. (avr_prologue_setup_frame): Use hard_frame_pointer_rtx. (expand_epilogue): Likewise. (avr_legitimize_address): Gut. (avr_legitimize_reload_address): Use hard_frame_pointer_rtx. (avr_hard_regno_nregs): New. (avr_hard_regno_ok): Allow only Pmode for arg and frame_pointers. (avr_regno_mode_code_ok_for_base_b): Handle arg and frame pointers. * config/avr/avr.h (FIXED_REGISTERS): Adjust arg pointer, add soft frame pointer. (CALL_USED_REGISTERS): Likewise. (REG_CLASS_CONTENTS): Likewise. (REGISTER_NAMES): Likewise. (HARD_REGNO_NREGS): Use avr_hard_regno_nregs. (HARD_FRAME_POINTER_REGNUM): New. (FRAME_POINTER_REGNUM): Use soft frame pointer. (ELIMINABLE_REGS): Eliminate from the soft frame pointer, remove the HARD_FRAME_POINTER self-elimination. Committed . Denis.
Re: [PATCH, AVR] Fix PR target/50925, use hard_frame_pointer_rtx
Denis Chertykov schrieb: Committed Denis Some questions regarding the fix: 1) You know if PR42204 is still relevant and can be closed? Or is it not related to PR50925? 2) Can we remove from avr.c:avr_option_override() the following: if (avr_strict_X) flag_caller_saves = 0; that hacked around similar spill fails? 3) As PR50775 is fixed: Would it make sense to turn on -mstrict-X per default now, i.e. no more fake X addressing except requested per -mno-strict-X? Johann
Re: [PATCH, AVR] Fix PR target/50925, use hard_frame_pointer_rtx
2012/1/13 Georg-Johann Lay a...@gjlay.de: Denis Chertykov schrieb: Committed Denis Some questions regarding the fix: 1) You know if PR42204 is still relevant and can be closed? Or is it not related to PR50925? The PR42204 is a duplicate of PR50925 or vice versa. But, I'm not sure that this bug can be closed because I don't fix the bug himself I just changed the port and now the port has a work around. 2) Can we remove from avr.c:avr_option_override() the following: if (avr_strict_X) flag_caller_saves = 0; that hacked around similar spill fails? 3) As PR50775 is fixed: Would it make sense to turn on -mstrict-X per default now, i.e. no more fake X addressing except requested per -mno-strict-X? This bug (and it's fix) isn't related to this addressing problems. Denis.
Re: [PATCH, AVR] Fix PR target/50925, use hard_frame_pointer_rtx
Denis Chertykov wrote: Hi Georg. I have found that conversion AVR port to using hard_frame_pointer have resolved PR 50925 . I have tested the patch without regressions, but I'm worry about it. Can you test it with your testsuite for regressions ? May be you have your own special difficult tests (special for addressing) ? Idea: In avr.c:avr_option_override() there is /* caller-save.c looks for call-clobbered hard registers that are assigned to pseudos that cross calls and tries so save-restore them around calls in order to reduce the number of stack slots needed. This might leads to situations where reload is no more able to cope with the challenge of AVR's very few address registers and fails to perform the requested spills. */ if (avr_strict_X) flag_caller_saves = 0; i.e. with -mstrict-X -fcaller-saves is turned off because I saw spill fails in test suite or building libs. This is a kludge, of course, to quick work around these spill fails. You can increase register pressure and register allocation stress considerably by turning on -mstrict-X per default: avr_strict_X = 1; Johann
[PATCH, AVR] Fix PR target/50925, use hard_frame_pointer_rtx
Hi Georg. I have found that conversion AVR port to using hard_frame_pointer have resolved PR 50925 . I have tested the patch without regressions, but I'm worry about it. Can you test it with your testsuite for regressions ? May be you have your own special difficult tests (special for addressing) ? 2012-01-09 Richard Henderson r...@redhat.com Denis Chertykov cherty...@gmail.com PR target/50925 * config/avr/avr-protos.h (avr_hard_regno_nregs): Declare. * config/avr/avr.c (avr_can_eliminate): Simplify. (avr_initial_elimination_offset): Likewise. (avr_prologue_setup_frame): Use hard_frame_pointer_rtx. (expand_epilogue): Likewise. (avr_legitimize_address): Gut. (avr_legitimize_reload_address): Use hard_frame_pointer_rtx. (avr_hard_regno_nregs): New. (avr_hard_regno_ok): Allow only Pmode for arg and frame_pointers. (avr_regno_mode_code_ok_for_base_b): Handle arg and frame pointers. * config/avr/avr.h (FIXED_REGISTERS): Adjust arg pointer, add soft frame pointer. (CALL_USED_REGISTERS): Likewise. (REG_CLASS_CONTENTS): Likewise. (REGISTER_NAMES): Likewise. (HARD_REGNO_NREGS): Use avr_hard_regno_nregs. (HARD_FRAME_POINTER_REGNUM): New. (FRAME_POINTER_REGNUM): Use soft frame pointer. (ELIMINABLE_REGS): Eliminate from the soft frame pointer, remove the HARD_FRAME_POINTER self-elimination. Denis. avr-sfp-patch Description: Binary data
Re: [PATCH, AVR] Fix PR target/50925, use hard_frame_pointer_rtx
Denis Chertykov wrote: Hi Georg. I have found that conversion AVR port to using hard_frame_pointer have resolved PR 50925 . I have tested the patch without regressions, but I'm worry about it. Can you test it with your testsuite for regressions ? May be you have your own special difficult tests (special for addressing) ? Hi Denis. My test suite is plain vanilla, there are no special test cases that are not upstream. So you should get the same test results.
