On Mon, 23 Oct 2017, Jakub Jelinek wrote:
> Hi!
>
> If all fails, when we can't prove that the PIC register is in some hard
> register, we delegitimize something + foo@GOTOFF as (something -
> _GLOBAL_OFFSET_TABLE_) + foo. That is reasonable for the middle-end to
> understand what is going on (it will never match in actual instructions
> though), unfortunately when trying to emit that into .debug_info section
> we run into the problem that .long _GLOBAL_OFFSET_TABLE_ etc. is not
> actually assembled as address of _GLOBAL_OFFSET_TABLE_, but as
> _GLOBAL_OFFSET_TABLE_-. (any time the assembler sees _GLOBAL_OFFSET_TABLE_
> symbol by name, it adds the special relocation) and thus we get a bogus
> expression.
>
> I couldn't come up with a way to express this that wouldn't be even larger
> than what we have, but if we actually not delegitimize it at all and let
> it be emitted as
> something
> .byte DW_OP_addr
> .long foo@GOTOFF
> .byte DW_OP_plus
> then it works fine and is even shorter than what we used to emit -
> something
> .byte DW_OP_addr
> .long _GLOBAL_OFFSET_TABLE_
> .byte DW_OP_minus
> .byte DW_OP_addr
> .long foo
> .byte DW_OP_plus
> In order to achieve that, we need to allow selected UNSPECs through
> into debug info, current trunk just gives up on all UNSPECs.
>
> Fortunately, we already have a hook for rejecting some constants, so
> by adding the rejection of all UNSPECs into the hook and on i386 overriding
> that hook we achieve what we want.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2017-10-23 Jakub Jelinek <ja...@redhat.com>
>
> PR debug/82630
> * target.def (const_not_ok_for_debug_p): Default to
> default_const_not_ok_for_debug_p instead of hook_bool_rtx_false.
> * targhooks.h (default_const_not_ok_for_debug_p): New declaration.
> * targhooks.c (default_const_not_ok_for_debug_p): New function.
> * dwarf2out.c (const_ok_for_output_1): Only reject UNSPECs for
> which targetm.const_not_ok_for_debug_p returned true.
> * config/arm/arm.c (arm_const_not_ok_for_debug_p): Return true
> for UNSPECs.
> * config/powerpcspe/powerpcspe.c (rs6000_const_not_ok_for_debug_p):
> Likewise.
> * config/rs6000/rs6000.c (rs6000_const_not_ok_for_debug_p): Likewise.
> * config/i386/i386.c (ix86_delegitimize_address_1): Don't delegitimize
> UNSPEC_GOTOFF with addend into addend - _GLOBAL_OFFSET_TABLE_ + symbol
> if !base_term_p.
> (ix86_const_not_ok_for_debug_p): New function.
> (i386_asm_output_addr_const_extra): Handle UNSPEC_GOTOFF.
> (TARGET_CONST_NOT_OK_FOR_DEBUG_P): Redefine.
>
> * g++.dg/guality/pr82630.C: New test.
>
> --- gcc/target.def.jj 2017-10-10 11:54:13.000000000 +0200
> +++ gcc/target.def 2017-10-20 14:07:06.463135128 +0200
> @@ -2822,7 +2822,7 @@ DEFHOOK
> "This hook should return true if @var{x} should not be emitted into\n\
> debug sections.",
> bool, (rtx x),
> - hook_bool_rtx_false)
> + default_const_not_ok_for_debug_p)
>
> /* Given an address RTX, say whether it is valid. */
> DEFHOOK
> --- gcc/targhooks.c.jj 2017-10-13 19:02:08.000000000 +0200
> +++ gcc/targhooks.c 2017-10-20 14:26:07.945464025 +0200
> @@ -177,6 +177,14 @@ default_legitimize_address_displacement
> return false;
> }
>
> +bool
> +default_const_not_ok_for_debug_p (rtx x)
> +{
> + if (GET_CODE (x) == UNSPEC)
What about UNSPEC_VOLATILE?
> + return true;
> + return false;
> +}
> +
> rtx
> default_expand_builtin_saveregs (void)
> {
> --- gcc/targhooks.h.jj 2017-10-13 19:02:08.000000000 +0200
> +++ gcc/targhooks.h 2017-10-20 14:26:07.945464025 +0200
> @@ -26,6 +26,7 @@ extern void default_external_libcall (rt
> extern rtx default_legitimize_address (rtx, rtx, machine_mode);
> extern bool default_legitimize_address_displacement (rtx *, rtx *,
> machine_mode);
> +extern bool default_const_not_ok_for_debug_p (rtx);
>
> extern int default_unspec_may_trap_p (const_rtx, unsigned);
> extern machine_mode default_promote_function_mode (const_tree, machine_mode,
> --- gcc/dwarf2out.c.jj 2017-10-19 16:18:44.000000000 +0200
> +++ gcc/dwarf2out.c 2017-10-20 14:39:49.432647598 +0200
> @@ -13740,9 +13740,17 @@ expansion_failed (tree expr, rtx rtl, ch
> static bool
> const_ok_for_output_1 (rtx rtl)
> {
> - if (GET_CODE (rtl) == UNSPEC)
> + if (targetm.const_not_ok_for_debug_p (rtl))
> {
> - /* If delegitimize_address couldn't do anything with the UNSPEC, assume
> + if (GET_CODE (rtl) != UNSPEC)
> + {
> + expansion_failed (NULL_TREE, rtl,
> + "Expression rejected for debug by the backend.\n");
> + return false;
> + }
> +
> + /* If delegitimize_address couldn't do anything with the UNSPEC, and
> + the target hook doesn't explicitly allow it in debug info, assume
> we can't express it in the debug info. */
> /* Don't complain about TLS UNSPECs, those are just too hard to
> delegitimize. Note this could be a non-decl SYMBOL_REF such as
> @@ -13769,13 +13777,6 @@ const_ok_for_output_1 (rtx rtl)
> return false;
> }
>
> - if (targetm.const_not_ok_for_debug_p (rtl))
> - {
> - expansion_failed (NULL_TREE, rtl,
> - "Expression rejected for debug by the backend.\n");
> - return false;
> - }
> -
> /* FIXME: Refer to PR60655. It is possible for simplification
> of rtl expressions in var tracking to produce such expressions.
> We should really identify / validate expressions
> --- gcc/config/arm/arm.c.jj 2017-10-19 16:18:44.000000000 +0200
> +++ gcc/config/arm/arm.c 2017-10-20 14:28:06.302046887 +0200
> @@ -30391,6 +30391,8 @@ arm_const_not_ok_for_debug_p (rtx p)
> tree decl_op0 = NULL;
> tree decl_op1 = NULL;
>
> + if (GET_CODE (p) == UNSPEC)
> + return true;
> if (GET_CODE (p) == MINUS)
> {
> if (GET_CODE (XEXP (p, 1)) == SYMBOL_REF)
> --- gcc/config/powerpcspe/powerpcspe.c.jj 2017-10-17 22:10:10.000000000
> +0200
> +++ gcc/config/powerpcspe/powerpcspe.c 2017-10-20 14:28:40.823633544
> +0200
> @@ -9536,6 +9536,8 @@ rs6000_delegitimize_address (rtx orig_x)
> static bool
> rs6000_const_not_ok_for_debug_p (rtx x)
> {
> + if (GET_CODE (x) == UNSPEC)
> + return true;
> if (GET_CODE (x) == SYMBOL_REF
> && CONSTANT_POOL_ADDRESS_P (x))
> {
> --- gcc/config/rs6000/rs6000.c.jj 2017-10-17 22:10:10.000000000 +0200
> +++ gcc/config/rs6000/rs6000.c 2017-10-20 14:29:01.066391168 +0200
> @@ -8985,6 +8985,8 @@ rs6000_delegitimize_address (rtx orig_x)
> static bool
> rs6000_const_not_ok_for_debug_p (rtx x)
> {
> + if (GET_CODE (x) == UNSPEC)
> + return true;
> if (GET_CODE (x) == SYMBOL_REF
> && CONSTANT_POOL_ADDRESS_P (x))
> {
> --- gcc/config/i386/i386.c.jj 2017-10-20 09:16:10.000000000 +0200
> +++ gcc/config/i386/i386.c 2017-10-20 14:53:19.520953612 +0200
> @@ -16657,13 +16657,17 @@ ix86_delegitimize_address_1 (rtx x, bool
> movl foo@GOTOFF(%ecx), %edx
> in which case we return (%ecx - %ebx) + foo
> or (%ecx - _GLOBAL_OFFSET_TABLE_) + foo if pseudo_pic_reg
> - and reload has completed. */
> + and reload has completed. Don't do the latter for debug,
> + as _GLOBAL_OFFSET_TABLE_ can't be expressed in the assembly. */
> if (pic_offset_table_rtx
> && (!reload_completed || !ix86_use_pseudo_pic_reg ()))
> result = gen_rtx_PLUS (Pmode, gen_rtx_MINUS (Pmode, copy_rtx
> (addend),
> pic_offset_table_rtx),
> result);
> - else if (pic_offset_table_rtx && !TARGET_MACHO && !TARGET_VXWORKS_RTP)
> + else if (base_term_p
> + && pic_offset_table_rtx
> + && !TARGET_MACHO
> + && !TARGET_VXWORKS_RTP)
> {
> rtx tmp = gen_rtx_SYMBOL_REF (Pmode, GOT_SYMBOL_NAME);
> tmp = gen_rtx_MINUS (Pmode, copy_rtx (addend), tmp);
> @@ -16716,6 +16720,25 @@ ix86_find_base_term (rtx x)
>
> return ix86_delegitimize_address_1 (x, true);
> }
> +
> +/* Return true if X shouldn't be emitted into the debug info.
> + Disallow UNSPECs other than @gotoff - we can't emit _GLOBAL_OFFSET_TABLE_
> + symbol easily into the .debug_info section, so we need not to
> + delegitimize, but instead assemble as @gotoff.
> + Disallow _GLOBAL_OFFSET_TABLE_ SYMBOL_REF - the assembler magically
> + assembles that as _GLOBAL_OFFSET_TABLE_-. expression. */
> +
> +static bool
> +ix86_const_not_ok_for_debug_p (rtx x)
> +{
> + if (GET_CODE (x) == UNSPEC && XINT (x, 1) != UNSPEC_GOTOFF)
> + return true;
> +
> + if (SYMBOL_REF_P (x) && strcmp (XSTR (x, 0), GOT_SYMBOL_NAME) == 0)
> + return true;
> +
> + return false;
> +}
>
> static void
> put_condition_code (enum rtx_code code, machine_mode mode, bool reverse,
> @@ -18032,6 +18055,10 @@ i386_asm_output_addr_const_extra (FILE *
> op = XVECEXP (x, 0, 0);
> switch (XINT (x, 1))
> {
> + case UNSPEC_GOTOFF:
> + output_addr_const (file, op);
> + fputs ("@gotoff", file);
> + break;
> case UNSPEC_GOTTPOFF:
> output_addr_const (file, op);
> /* FIXME: This might be @TPOFF in Sun ld. */
> @@ -49415,6 +49442,9 @@ ix86_run_selftests (void)
> #undef TARGET_DELEGITIMIZE_ADDRESS
> #define TARGET_DELEGITIMIZE_ADDRESS ix86_delegitimize_address
>
> +#undef TARGET_CONST_NOT_OK_FOR_DEBUG_P
> +#define TARGET_CONST_NOT_OK_FOR_DEBUG_P ix86_const_not_ok_for_debug_p
> +
> #undef TARGET_MS_BITFIELD_LAYOUT_P
> #define TARGET_MS_BITFIELD_LAYOUT_P ix86_ms_bitfield_layout_p
>
> --- gcc/testsuite/g++.dg/guality/pr82630.C.jj 2017-10-20 15:23:37.978128740
> +0200
> +++ gcc/testsuite/g++.dg/guality/pr82630.C 2017-10-20 15:30:39.608050350
> +0200
> @@ -0,0 +1,58 @@
> +// PR debug/82630
> +// { dg-do run }
> +// { dg-additional-options "-fPIC" { target fpic } }
> +
> +struct C
> +{
> + int &c;
> + long d;
> + __attribute__((always_inline)) C (int &x) : c(x), d() {}
> +};
> +int v;
> +
> +__attribute__((noipa)) void
> +fn1 (const void *x)
> +{
> + asm volatile ("" : : "g" (x) : "memory");
> +}
> +
> +__attribute__((noipa)) void
> +fn2 (C x)
> +{
> + int a = x.c + x.d;
> + asm volatile ("" : : "g" (a) : "memory");
> +}
> +
> +__attribute__((noipa)) void
> +fn3 (void)
> +{
> + asm volatile ("" : : : "memory");
> +}
> +
> +__attribute__((noipa))
> +#ifdef __i386__
> +__attribute__((regparm (2)))
> +#endif
> +static void
> +fn4 (int *x, const char *y, C z)
> +{
> + fn2 (C (*x));
> + fn1 ("baz");
> + fn2 (z); // { dg-final { gdb-test 41 "y\[0\]" "'f'" } }
> + fn1 ("baz"); // { dg-final { gdb-test 41 "y\[1\]" "'o'" } }
> +}
> +
> +__attribute__((noipa)) void
> +fn5 (int *x)
> +{
> + fn4 (x, "foo", C (*x));
> + fn3 ();
> +}
> +
> +int
> +main ()
> +{
> + int a = 10;
> + fn5 (&a);
> + return 0;
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
