On Tue, 2023-07-18 at 11:04 -0400, Vladimir Makarov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On 7/17/23 03:17, senthilkumar.selva...@microchip.com wrote:
> > On Fri, 2023-07-14 at 09:29 -0400, Vladimir Makarov wrote:
> > > If you send me the preprocessed test, I could start to work on it to fix
> > > the problems. I think it is hard to fix them right for a person having
> > > a little experience with LRA.
> > >
> > >
> > Ok, this is a reduced test case that reproduces the failure.
> >
> > $ cat case.c
> > typedef int HItype __attribute__ ((mode (HI)));
> > HItype
> > __mulvhi3 (HItype a, HItype b)
> > {
> > HItype w;
> >
> > if (__builtin_mul_overflow (a, b, &w))
> > __builtin_trap ();
> >
> > return w;
> > }
> >
> > On latest master, this trivial patch turns on LRA for avr
> > --- gcc/config/avr/avr.cc
> > +++ gcc/config/avr/avr.cc
> > @@ -15244,9 +15244,6 @@ avr_float_lib_compare_returns_bool (machine_mode
> > mode, enum rtx_code)
> > #undef TARGET_CONVERT_TO_TYPE
> > #define TARGET_CONVERT_TO_TYPE avr_convert_to_type
> >
> > -#undef TARGET_LRA_P
> > -#define TARGET_LRA_P hook_bool_void_false
> > -
> > #undef TARGET_ADDR_SPACE_SUBSET_P
> > #define TARGET_ADDR_SPACE_SUBSET_P avr_addr_space_subset_p
> >
> > Then configuring and building for avr without attempting to build libgcc
> >
> > $ configure --target=avr --prefix=<prefix_dir> --enable-languages=c && make
> > all-host && make install-host
> >
> > And finally to reproduce the failure
> > $ <prefix_dir>/bin/avr-gcc -mmcu=avr25 case.c -Os
>
> Thank you. I've reproduced the bug and started to work on it
> yesterday. The problem is a bit tricky than I initially thought but I
> believe I'll fix it on this week.
>
>
Hi Vlad,
I can confirm your commit
(https://gcc.gnu.org/git?p=gcc.git;a=commit;h=2971ff7b1d564ac04b537d907c70e6093af70832)
fixes the above problem, thank you. However, I see execution failures if a
pseudo assigned to FP has to be spilled because of stack slot creation.
To reproduce, build the compiler just like above, and then do
$ avr-gcc -mmcu=avr51
<gcc-src-dir>/gcc/testsuite/gcc.c-torture/execute/20050224-1.c -O2 -S
-fdump-rtl-all
The execution failure occurs at this point
movw r24,r2
sbiw r24,36
brne .L8
r2 is never set anywhere at all in the assembly.
The relevant insns (in the IRA dump) are
(insn 3 15 4 3 (set (reg/v:HI 51 [ j ])
(const_int 0 [0]))
"gcc/gcc/testsuite/gcc.c-torture/execute/20050224-1.c":19:21 101 {*movhi_split}
(expr_list:REG_EQUAL (const_int 0 [0])
(nil)))
...
(insn 28 27 67 8 (parallel [
(set (reg/v:HI 51 [ j ])
(plus:HI (reg/v:HI 51 [ j ])
(const_int 1 [0x1])))
(clobber (scratch:QI))
])
"/home/i41766/code/personal/gcc/gcc/testsuite/gcc.c-torture/execute/20050224-1.c":28:8
175 {addhi3_clobber}
(nil))
...
(jump_insn 44 43 45 13 (parallel [
(set (pc)
(if_then_else (ne (reg/v:HI 51 [ j ])
(const_int 36 [0x24]))
(label_ref:HI 103)
(pc)))
(clobber (scratch:QI))
])
"/home/i41766/code/personal/gcc/gcc/testsuite/gcc.c-torture/execute/20050224-1.c":11:16
discrim 1 713 {cbranchhi4_insn}
(expr_list:REG_DEAD (reg/v:HI 51 [ j ])
(int_list:REG_BR_PROB 7 (nil)))
-> 103)
LRA deletes insns 3 and 28, and uses r2 in the jump_insn.
In the reload dump, for pseudo r51, I'm seeing this
subreg regs:
Frame pointer can not be eliminated anymore
Spilling non-eliminable hard regs: 28 29
Spilling r51(28)
Slot 0 regnos (width = 0): 46
Slot 1 regnos (width = 0): 45
lra_update_fp2sp_elimination calls spill_pseudos with
HARD_FRAME_POINTER_REGNUM, and that sets reg_renumber[51] to -1.
Later down the line, process_bb_lives is called with dead_insn_p=true from
lra_create_lives_ranges_1 on the relevant BB (#8), and df_get_live_out
on that BB does not contain 51 (even though previous calls to the same BB
did).
Breakpoint 8, process_bb_lives (bb=0x7fffea570240, curr_point=@0x7fffffffd838:
25, dead_insn_p=true) at gcc/gcc/lra-lives.cc:664
664 function_abi last_call_abi = default_function_abi;
(gdb) n
666 reg_live_out = df_get_live_out (bb);
(gdb)
667 sparseset_clear (pseudos_live);
(gdb) p debug_bitmap(reg_live_out)
first = 0x321c128 current = 0x321c128 indx = 0
0x321c128 next = (nil) prev = (nil) indx = 0
bits = { 28 32 34 43 44 47 48 49 50 }
process_bb_lives then considers the insn setting 51 (and
the reload insns LRA created) as dead, and removes them.
BB 8
Insn 67: point = 31, n_alt = -1
Insn 114: point = 31, n_alt = 3
Deleting dead insn 114
deleting insn with uid = 114.
Insn 28: point = 31, n_alt = 1
Deleting dead insn 28
deleting insn with uid = 28.
Insn 113: point = 31, n_alt = 2
Deleting dead insn 113
Same for insn 3 as well
BB 3
Insn 92: point = 40, n_alt = -1
Insn 5: point = 40, n_alt = 1
Insn 4: point = 41, n_alt = 3
Insn 3: point = 42, n_alt = 3
Deleting dead insn 3
deleting insn with uid = 3.
Yet when it prints "Global pseudo live data has been updated" after
all this, r51 is live again :(
BB 8:
livein: 8:
43 44 47 48 49 50 51
liveout: 8:
28 32 34 43 44 47 48 49 50 51
Eventually, it assigns 2 to r51, resulting in just the compare and
branch instruction remaining in the assembly.
Is this an LRA bug or is the target doing something wrong?
Regards
Senthil
