public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "jakub at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug target/115324] [12/13/14/15 Regression] PCH of rs6000 builtins broken
Date: Mon, 03 Jun 2024 09:09:12 +0000	[thread overview]
Message-ID: <bug-115324-4-UY72T12QQo@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-115324-4@http.gcc.gnu.org/bugzilla/>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115324

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
   Target Milestone|---                         |12.4
            Summary|[12/13/14/15 Regression]    |[12/13/14/15 Regression]
                   |PCH (and maybe GC) of       |PCH of rs6000 builtins
                   |rs6000 builtins broken      |broken
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2024-06-03

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
While for GC everything looks ok, in particular we have
  {
    &rs6000_instance_info[0].fntype,
    1 * (RS6000_INST_MAX),
    sizeof (rs6000_instance_info[0]),
    &gt_ggc_mx_tree_node,
    &gt_pch_nx_tree_node
  },
  {
    &rs6000_builtin_info[0].fntype,
    1 * (RS6000_BIF_MAX),
    sizeof (rs6000_builtin_info[0]),
    &gt_ggc_mx_tree_node,
    &gt_pch_nx_tree_node
  },
GC roots which are strided so that they cover just the fntype fields in
rs6000_instance_info and rs6000_builtin_info, it causes problems for PCH,
because
PCH saves/restores the whole rs6000_instance_info/rs6000_builtin_info arrays,
which are huge and worse the contain quite a lot of pointers to .rodata/.data
sections besides the tree fntype members.
In struct bifdata it is
  const char *GTY((skip(""))) bifname;
...
  const char *GTY((skip(""))) attr_string;
and in struct ovlddata
  const char *GTY((skip(""))) bifname;
...
  ovlddata *GTY((skip(""))) next;
fields.  bifname and attr_string members point to string literals, so .rodata
section,
and next is either NULL or &rs6000_instance_info[RS6000_INST_XXX]
Now, the problem is when cc1/cc1plus etc. are PIEs, the saving is done when
.rodata/.data sections are loaded at one address, but the restoring can be done
when they are loaded at different address, so the pointers are garbage after
PCH restore.

We could extend GTY, to have a similar flag to callback where we'd somehow fix
up those pointers in global GTY roots.
Or I think we can just move the fntype members from those structures to
separate arrays, making the original arrays non-GTY and only GTY the new
*_fntype arrays.

I've implemented the latter, will attach it momentarily.
It seems to decrease the size of .data arrays.
                             vanilla    patched
rs6000_builtin_info          130832     110704
rs6000_instance_info          81568      40784
rs6000_overload_info           7392       7392
rs6000_builtin_info_fntype        0      10064
rs6000_instance_info_fntype       0      20392
sum                          219792     189336

On a cross from x86_64 -> powerpc64le it increases .text size in the
_Z30rs6000_init_generated_builtinsv function, but will need to find out if that
is also the case on powerpc64le native where section anchors are used, the
changes in the source are like:
   rs6000_overload_info[RS6000_OVLD_VEC_VSUBFP - base].first_instance
-    = &rs6000_instance_info[RS6000_INST_VSUBFP_DEPR1];
+    = RS6000_INST_VSUBFP_DEPR1;

-  rs6000_instance_info[RS6000_INST_VSUBSBS_DEPR1].fntype
+  rs6000_instance_info_fntype[RS6000_INST_VSUBSBS_DEPR1]
     = v16qi_ftype_v16qi_v16qi;
and
-  rs6000_builtin_info[RS6000_BIF_CFSTRING].fntype
+  rs6000_builtin_info_fntype[RS6000_BIF_CFSTRING]
     = v_ftype_v;
so there is IMO nothing inherently larger.

  reply	other threads:[~2024-06-03  9:09 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-03  8:44 [Bug target/115324] New: [12/13/14/15 Regression] PCH (and maybe GC) " jakub at gcc dot gnu.org
2024-06-03  9:09 ` jakub at gcc dot gnu.org [this message]
2024-06-03  9:16 ` [Bug target/115324] [12/13/14/15 Regression] PCH " jakub at gcc dot gnu.org
2024-06-03 21:12 ` cvs-commit at gcc dot gnu.org
2024-06-04 14:26 ` cvs-commit at gcc dot gnu.org
2024-06-04 14:29 ` [Bug target/115324] [12/13 " jakub at gcc dot gnu.org
2024-06-11  6:17 ` cvs-commit at gcc dot gnu.org
2024-06-11 10:38 ` cvs-commit at gcc dot gnu.org
2024-06-11 10:54 ` jakub at gcc dot gnu.org
2024-06-18  6:33 ` cvs-commit at gcc dot gnu.org

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bug-115324-4-UY72T12QQo@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).