[PATCH] Fix -Wshadow=local warnings in rtl.h

[Jakub Jelinek]

On Sat, Oct 05, 2019 at 06:12:37AM +0000, Bernd Edlinger wrote:
> On 10/3/19 5:25 PM, Jakub Jelinek wrote:
> > What effect does this have on the cc1/cc1plus .text sizes?
> 
> r276457:
> 
> with patch, --enable-languages=all --enable-checking=yes,rtl
> $ size gcc/cc1
>    text    data     bss     dec     hex filename
> 35117708          50984 1388192 36556884        22dd054 gcc/cc1
> $ size gcc/cc1plus
>    text    data     bss     dec     hex filename
> 37871569          54640 1391936 39318145        257f281 gcc/cc1plus
> 
> unpatched, --enable-languages=all --enable-checking=yes,rtl
> $ size gcc/cc1
>    text    data     bss     dec     hex filename
> 36972031          50984 1388192 38411207        24a1bc7 gcc/cc1
> $ size gcc/cc1plus
>    text    data     bss     dec     hex filename
> 39725980          54640 1391936 41172556        2743e4c gcc/cc1plus
> 
> I am a bit surprised, that the patch gives smaller code. I used not the original
> Version of this patch, but the one based on Richard's suggestion.

In that case, can you check if all the non-failed checks are actually
inlined?  We do want to inline the if (x->code != 123) check_failed (...);
and don't want to inline the check_failed (...).  With partial inlining, it
might be ok if we inline the if and don't inline the actual caller of
check_failed, but I'd worry about checks for multiple codes if we inline
from if (x->code != 123 && x->code != 235 && x->code != 348) inline say
just x->code != 123 or x->code != 123 && x->code != 235 and leave the rest
in the out of line function.

> > Does this affect debuggability of --enable-checking=yes,rtl compilers?
> > I mean, often when we replace some macros with inlines step in GDB
> > becomes a bigger nightmare, having to go through tons of inline frames.
> > gdbinit.in has a lengthy list of inlines to skip in rtl.h, shouldn't this be
> > added to that list?  Not 100% sure how well it will work on rtl checking
> > vs. non-rtl checking builds.
> > 
> 
> I don't see a big problem here.  If I type "s" in gdb it jumps to the check
> function and the next s jumps back, adding skip instructions in gdbinit.in
> does not seem to have any effect for me, but the debug is not that uncomfortable
> anyway.
> 
> Interesting is that gdb also jumps in the check function when I press n.
> That is also Independent of the gdbinit.in, seems to be a bug due to inlining
> code from another line than the original macro, but nothing terrilby bad.

Unfortunately, for me the above two counts as terribly bad, show
stopper here.  A lot of function calls in RTL are like:
rtx_equal_for_memref_p (XEXP (x, 0), XEXP (y, 0))
rtx_equal_p (ENTRY_VALUE_EXP (x), ENTRY_VALUE_EXP (y))
force_operand (XEXP (dest_mem, 0), target)
etc.  (just random examples), during debugging one is absolutely
uninterested in stepping into the implementation of XEXP, you know what it
means, you want to go stright into the rtx_equal_for_memref_p etc.  call. 
It is already bad that one has to step through the poly_int* stuff or
rhs_regno for REGNO (we should add rhs_regno to gdbinit.in and some of the
poly_int* stuff too).  And no, one can't use n instead of s, because then
the whole call is skipped.  b rtx_equal_for_memref_p; n works, but it is
time consuming and one needs to delete the breakpoint again.

	Jakub