[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-24 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

Jakub Jelinek  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #37 from Jakub Jelinek  ---
Fixed.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-14 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #36 from GCC Commits  ---
The master branch has been updated by Jakub Jelinek :

https://gcc.gnu.org/g:74ea20e16cf18b42071557b71a42ea31c8192425

commit r15-7526-g74ea20e16cf18b42071557b71a42ea31c8192425
Author: Jakub Jelinek 
Date:   Fri Feb 14 12:01:13 2025 +0100

tree: Fix up the DECL_VALUE_EXPR GC marking [PR118790]

The ggc_set_mark call in gt_value_expr_mark_2 is actually wrong, that
just marks the VAR_DECL itself, but doesn't mark the subtrees of it (type
etc.).  So, I think we need to test gcc_marked_p for whether it is marked
or not, if not marked walk the DECL_VALUE_EXPR and then gt_ggc_mx mark
the VAR_DECL that was determined not marked and needs to be marked now.
One option would be to call gt_ggc_mx (t) right after the DECL_VALUE_EXPR
walking, but I'm a little bit worried that the subtree marking could mark
other VAR_DECLs (e.g. seen from DECL_SIZE or TREE_TYPE and the like) and
if they would be DECL_HAS_VALUE_EXPR_P we might not walk their
DECL_VALUE_EXPR anymore later.
So, the patch defers the gt_ggc_mx calls until we've walked all the
DECL_VALUE_EXPRs directly or indirectly connected to already marked
VAR_DECLs.

2025-02-14  Jakub Jelinek  

PR debug/118790
* tree.cc (struct gt_value_expr_mark_data): New type.
(gt_value_expr_mark_2): Don't call ggc_set_mark, instead check
ggc_marked_p.  Treat data as gt_value_expr_mark_data * with pset
in it rather than address of the pset itself and push to be marked
VAR_DECLs into to_mark vec.
(gt_value_expr_mark_1): Change argument from hash_set *
to gt_value_expr_mark_data * and find pset in it.
(gt_value_expr_mark): Pass to traverse_noresize address of
gt_value_expr_mark_data object rather than hash_table and
for all entries in the to_mark vector after the traversal call
gt_ggc_mx.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-13 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #35 from GCC Commits  ---
The master branch has been updated by Jakub Jelinek :

https://gcc.gnu.org/g:7738c6286fba7ec2112823f53cc2cefac2c8d007

commit r15-7506-g7738c6286fba7ec2112823f53cc2cefac2c8d007
Author: Jakub Jelinek 
Date:   Thu Feb 13 14:14:50 2025 +0100

tree, gengtype: Fix up GC issue with DECL_VALUE_EXPR [PR118790]

The following testcase ICEs, because we have multiple levels of
DECL_VALUE_EXPR VAR_DECLs:
  character(kind=1) id_string[1:.id_string] [value-expr: *id_string.55];
  character(kind=1)[1:.id_string] * id_string.55 [value-expr:
FRAME.107.id_string.55];
  integer(kind=8) .id_string [value-expr: FRAME.107..id_string];
id_string is the user variable mentioned in BLOCK_VARS, it has
DECL_VALUE_EXPR because it is a VLA, id_string.55 is a temporary created by
gimplify_vla_decl as the address that points to the start of the VLA, what
is normally used in the IL to access it.  But as this artificial var is
then
used inside of a nested function, tree-nested.cc adds DECL_VALUE_EXPR to it
too and moves the actual value into the FRAME.107 object's member.
Now, remove_unused_locals removes id_string.55 (and various other
VAR_DECLs)
from cfun->local_decls, simply because it is not mentioned in the IL at all
(neither is id_string itself, but that is kept in BLOCK_VARS as it has
DECL_VALUE_EXPR).  So, after this point, id_string.55 tree isn't referenced
from
anywhere but id_string's DECL_VALUE_EXPR.  Next GC collection is triggered,
and we are unlucky enough that in the value_expr_for_decl hash table
(underlying hash map for DECL_VALUE_EXPR) the id_string.55 entry comes
before the id_string entry.  id_string is ggc_marked_p because it is
referenced from BLOCK_VARS, but id_string.55 is not, as we don't mark
DECL_VALUE_EXPR anywhere but by gt_cleare_cache on value_expr_for_decl.
But gt_cleare_cache does two things, it calls clear_slots on entries
where the key is not ggc_marked_p (so the id_string.55 mapping to
FRAME.107.id_string.55 is lost and DECL_VALUE_EXPR (id_string.55) becomes
NULL) but then later we see id_string entry, which is ggc_marked_p, so mark
the whole hash table entry, which sets ggc_set_mark on id_string.55.  But
at this point its DECL_VALUE_EXPR is lost.
Later during dwarf2out.cc we want to emit DW_AT_location for id_string, see
it has DECL_VALUE_EXPR, so emit it as indirection of id_string.55 for which
we again lookup DECL_VALUE_EXPR as it has DECL_HAS_VALUE_EXPR_P, but as it
is NULL, we ICE, instead of finding it is a subobject of FRAME.107 for
which
we can find its stack location.

Now, as can be seen in the PR, I've tried to tweak tree-ssa-live.cc so that
it would keep id_string.55 in cfun->local_decls; that prohibits it from
the DECL_VALUE_EXPR of it being GC until expansion, but then we shrink and
free cfun->local_decls completely and so GC at that point still can throw
it away.

The following patch adds an extension to the GTY ((cache)) option, before
calling the gt_cleare_cache on some hash table by specifying
GTY ((cache ("somefn"))) it calls somefn on that hash table as well.
And this extra hook can do any additional ggc_set_mark needed so that
gt_cleare_cache preserves everything that is actually needed and throws
away the rest.

In order to make it just 2 pass rather than up to n passes - (if we had
say
id1 -> something, id2 -> x(id1), id3 -> x(id2), id4 -> x(id3), id5 ->
x(id4)
in the value_expr_for_decl hash table in that order (where idN are
VAR_DECLs
with DECL_HAS_VALUE_EXPR_P, id5 is the only one mentioned from outside and
idN -> X stands for idN having DECL_VALUE_EXPR X, something for some
arbitrary tree and x(idN) for some arbitrary tree which mentions idN
variable) and in each pass just marked the to part of entries with
ggc_marked_p base.from we'd need to repeat until we don't mark anything)
the patch calls walk_tree on DECL_VALUE_EXPR of the marked trees and if it
finds yet unmarked tree, it marks it and walks its DECL_VALUE_EXPR as well
the same way.

2025-02-13  Jakub Jelinek  

PR debug/118790
* gengtype.cc (write_roots): Remove cache variable, instead break
from
the loop on match and test o for NULL.  If the cache option has
non-empty string argument, call the specified function with v->name
as argument before calling gt_cleare_cache on it.
* tree.cc (gt_value_expr_mark_2, gt_value_expr_mark_1,
gt_value_expr_mark): New functions.
(value_expr_for_decl): Use GTY ((cache ("gt_value_expr_mark")))
rather
than just GTY ((cache)).
* doc/gty.texi (cache): Document optional argument of cache option.

* gfortran.dg/gomp/pr118790.f90: New test.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-12 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

Jakub Jelinek  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |jakub at gcc dot gnu.org

--- Comment #34 from Jakub Jelinek  ---
Created attachment 60478
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=60478&action=edit
gcc15-pr118790.patch

Full untested patch.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-12 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #33 from Jakub Jelinek  ---
So, for the GTY fix I was thinking about something like:
--- gcc/tree.cc.jj  2025-01-20 10:26:42.216048422 +0100
+++ gcc/tree.cc 2025-02-12 13:31:07.912605405 +0100
@@ -217,7 +217,7 @@ static GTY ((cache)) hash_table *debug_expr_for_decl;

-static GTY ((cache))
+static GTY ((cache ("2pass")))
  hash_table *value_expr_for_decl;

 static GTY ((cache))
--- gcc/hash-map.h.jj   2025-01-02 11:23:37.818219480 +0100
+++ gcc/hash-map.h  2025-02-12 13:33:08.003941869 +0100
@@ -308,6 +308,7 @@ private:
   template friend void gt_pch_nx
(hash_map *);
   template friend void gt_pch_nx
(hash_map *, gt_pointer_operator, void *);
   template friend void gt_cleare_cache
(hash_map *);
+  template friend void
gt_cleare_cache_2pass (hash_map *);

   hash_table m_table;
 };
@@ -337,6 +338,14 @@ gt_cleare_cache (hash_map *h)
 }

 template
+inline void
+gt_cleare_cache_2pass (hash_map *h)
+{
+  if (h)
+gt_cleare_cache_2pass (&h->m_table);
+}
+
+template
 inline void
 gt_pch_nx (hash_map *h, gt_pointer_operator op, void *cookie)
 {
--- gcc/gengtype.cc.jj  2025-01-02 11:23:02.613710956 +0100
+++ gcc/gengtype.cc 2025-02-12 13:25:02.306650140 +0100
@@ -4656,13 +4656,12 @@ write_roots (pair_p variables, bool emit
   outf_p f = get_output_file_with_visibility (CONST_CAST (input_file*,
  v->line.file));
   struct flist *fli;
-  bool cache = false;
   options_p o;

   for (o = v->opt; o; o = o->next)
if (strcmp (o->name, "cache") == 0)
- cache = true;
-   if (!cache)
+ break;
+   if (!o)
continue;

   for (fli = flp; fli; fli = fli->next)
@@ -4677,7 +4676,10 @@ write_roots (pair_p variables, bool emit
  oprintf (f, " ()\n{\n");
}

-  oprintf (f, "  gt_cleare_cache (%s);\n", v->name);
+  if (o->kind == OPTION_STRING && strcmp (o->info.string, "2pass") == 0)
+   oprintf (f, "  gt_cleare_cache_2pass (%s);\n", v->name);
+  else
+   oprintf (f, "  gt_cleare_cache (%s);\n", v->name);
 }

   finish_cache_funcs (flp);
--- gcc/hash-table.h.jj 2025-01-02 11:23:18.432490116 +0100
+++ gcc/hash-table.h2025-02-12 14:05:06.137219383 +0100
@@ -524,6 +524,7 @@ private:
  gt_pointer_operator, void *);

   template friend void gt_cleare_cache (hash_table *);
+  template friend void gt_cleare_cache_2pass (hash_table *);

   void empty_slow ();

@@ -1318,4 +1319,26 @@ gt_cleare_cache (hash_table *h)
   }
 }

+/* Variant of the above for cache ("2pass").  This version first marks
+   all the cache entries which should be kept and only afterwards deletes
+   those that shouldn't, which allows some keys in the cache to be marked
+   only when referenced in values of other entries in the cache.  */
+
+template
+inline void
+gt_cleare_cache_2pass (hash_table *h)
+{
+  typedef hash_table table;
+  if (!h)
+return;
+
+  for (typename table::iterator iter = h->begin (); iter != h->end (); ++iter)
+if (!table::is_empty (*iter) && !table::is_deleted (*iter))
+  {
+   int res = H::keep_cache_entry (*iter);
+   if (res != 0 && res != -1)
+ gt_ggc_mx ((*iter)->to);
+  }
+  gt_cleare_cache (h);
+}
 #endif /* TYPED_HASHTAB_H */

This fixes the ICE, but now that I think about that, 2 passes aren't enough the
way it is written, one pass will handle just one extra indirection through
DECL_VALUE_EXPR (like this testcase, id_string ggc_marked_p, which has
*id_string.55 has DECL_VALUE_EXPR and id_string.55 not initially marked, and
id_string.55 having DECL_VALUE_EXPR to some tree referencing just VAR_DECLs
already marked.  But if there is an extra indirection and one is unlucky,
ggc_marked_p x could have DECL_VALUE_EXPR *y ad y could have DECL_VALUE_EXPR *z
and z could have DECL_VALUE_EXPR *w, then if
tree_decl_map entry for z comes first, then for y and then for x, this would
mark also y but not already z.
So, probably instead of gt_ggc_mx ((*iter)->to); it should call some function
(perhaps specified in cache's operand) on *iter and that function should
actually walk_tree &e.to with callback that would for any VAR_DECLs
!ggc_marked_p with DECL_HAS_VALUE_EXPR_P first walk their DECL_VALUE_EXPR and
then gt_ggc_mx mark them.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-12 Thread rguenther at suse dot de via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #32 from rguenther at suse dot de  ---
On Wed, 12 Feb 2025, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790
> 
> --- Comment #31 from Jakub Jelinek  ---
> (In reply to rguent...@suse.de from comment #30)
> > > Note, normally these VLA addresses don't have DECL_VALUE_EXPR and so are
> > > usually used in the IL.  It is just that tree-nested.cc in this case added
> > > DECL_VALUE_EXPR on those because they are (or were) referenced from nested
> > > function.
> > 
> > I think the intent for this is to reflect this into debug info.  So,
> > ... I think tree-nested.cc should put amend the related VAR which should
> > be already somewhere in BLOCK_VARS?
> 
> Note, moving it to BLOCK_VARS is actually wrong, the VAR_DECL is
> DECL_ARTIFICIAL, but !DECL_IGNORED_P - we actually want to be able to use its
> value.
> We don't want users to see id_string.55 in the debugger and be able to ask its
> value etc.  We just want users to look at id_string which is a VLA.
> So, from dwarf2out.cc POV, we want to know where id_string lives and go there
> through
> the the DECL_VALUE_EXPRs
>   character(kind=1) id_string[1:.id_string] [value-expr: *id_string.55];
>   character(kind=1)[1:.id_string] * id_string.55 [value-expr:
> FRAME.107.id_string.55];
>   integer(kind=8) .id_string [value-expr: FRAME.107..id_string];
> to actual RTL and DWARF expressions for that.
> So, conceptually I think we want to keep current behavior but ensure that
> chains of DECL_VALUE_EXPRs still work after GC.
> One way to achieve that could be some special GTY extension which does the 2
> pass swipes over the hash_map rather than one pass (could say GTY((user)) deal
> with that?).
> And another possibility would be to "inline" the DECL_VALUE_EXPR subtrees, 
> when
> we note that id_string DECL_VALUE_EXPR refers to id_string.55 which is not in
> any BLOCK_VARS and has DECL_VALUE_EXPR, simply take its DECL_VALUE_EXPR and
> replace id_string.55 with that in DECL_VALUE_EXPR of id_string, etc.

Hmm, so CFG expand pruning all local_decls interferes with GC then.  Can
we avoid pruning it completely?  Two-stage handling of cache hash tables
would work and is probably a safe fix, but then it also feels somewhat
expensive (IMO with checking we should assert we're _not_ getting any
unmarked entries marked with this approach).

Inlining DECL_VALUE_EXPR is an interesting idea, but it feels like
unchartered territory at this point.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-12 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #31 from Jakub Jelinek  ---
(In reply to rguent...@suse.de from comment #30)
> > Note, normally these VLA addresses don't have DECL_VALUE_EXPR and so are
> > usually used in the IL.  It is just that tree-nested.cc in this case added
> > DECL_VALUE_EXPR on those because they are (or were) referenced from nested
> > function.
> 
> I think the intent for this is to reflect this into debug info.  So,
> ... I think tree-nested.cc should put amend the related VAR which should
> be already somewhere in BLOCK_VARS?

Note, moving it to BLOCK_VARS is actually wrong, the VAR_DECL is
DECL_ARTIFICIAL, but !DECL_IGNORED_P - we actually want to be able to use its
value.
We don't want users to see id_string.55 in the debugger and be able to ask its
value etc.  We just want users to look at id_string which is a VLA.
So, from dwarf2out.cc POV, we want to know where id_string lives and go there
through
the the DECL_VALUE_EXPRs
  character(kind=1) id_string[1:.id_string] [value-expr: *id_string.55];
  character(kind=1)[1:.id_string] * id_string.55 [value-expr:
FRAME.107.id_string.55];
  integer(kind=8) .id_string [value-expr: FRAME.107..id_string];
to actual RTL and DWARF expressions for that.
So, conceptually I think we want to keep current behavior but ensure that
chains of DECL_VALUE_EXPRs still work after GC.
One way to achieve that could be some special GTY extension which does the 2
pass swipes over the hash_map rather than one pass (could say GTY((user)) deal
with that?).
And another possibility would be to "inline" the DECL_VALUE_EXPR subtrees, when
we note that id_string DECL_VALUE_EXPR refers to id_string.55 which is not in
any BLOCK_VARS and has DECL_VALUE_EXPR, simply take its DECL_VALUE_EXPR and
replace id_string.55 with that in DECL_VALUE_EXPR of id_string, etc.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-12 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #29 from Jakub Jelinek  ---
Perhaps with a help of yet another bitmap, mark_scope_block_unused could
bitmap_set_bit DECL_UID of all the BLOCK_VARS VAR_DECLs with
DECL_HAS_VALUE_EXPR_P seen in the scopes, and then remove_unused_scope_block_p
could when keeping such VAR_DECLs walk_tree their DECL_VALUE_EXPR and if it
sees in there a VAR_DECL with DECL_HAS_VALUE_EXPR_P which is not marked in that
bitmap, mark it there and queue for addition into the outermost BLOCK_VARS.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-12 Thread rguenther at suse dot de via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #30 from rguenther at suse dot de  ---
On Wed, 12 Feb 2025, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790
> 
> --- Comment #28 from Jakub Jelinek  ---
> --- gcc/tree-ssa-live.cc.jj 2025-02-12 11:03:53.607556187 +0100
> +++ gcc/tree-ssa-live.cc2025-02-12 11:54:34.779131237 +0100
> @@ -369,9 +369,17 @@ mark_all_vars_used_1 (tree *tp, int *wal
>  {
>/* When a global var becomes used for the first time also walk its
>   initializer (non global ones don't have any).  */
> -  if (set_is_used (t) && is_global_var (t)
> - && DECL_CONTEXT (t) == current_function_decl)
> -   mark_all_vars_used (&DECL_INITIAL (t));
> +  if (set_is_used (t))
> +   {
> + if (is_global_var (t)
> + && DECL_CONTEXT (t) == current_function_decl)
> +   mark_all_vars_used (&DECL_INITIAL (t));
> + if (DECL_HAS_VALUE_EXPR_P (t))
> +   {
> + tree dve = DECL_VALUE_EXPR (t);
> + mark_all_vars_used (&dve);
> +   }
> +   }
>  }
>/* remove_unused_scope_block_p requires information about labels
>   which are not DECL_IGNORED_P to tell if they might be used in the IL.  
> */
> @@ -389,7 +397,9 @@ mark_all_vars_used_1 (tree *tp, int *wal
>  }
> 
>  /* Mark the scope block SCOPE and its subblocks unused when they can be
> -   possibly eliminated if dead.  */
> +   possibly eliminated if dead.  Also, as remove_unused_scope_block_p is 
> going
> +   to keep all BLOCK_VARS VAR_DECLs with DECL_HAS_VALUE_EXPR_P, mark all vars
> +   their DECL_VALUE_EXPR refers to.  */
> 
>  static void
>  mark_scope_block_unused (tree scope)
> @@ -398,6 +408,12 @@ mark_scope_block_unused (tree scope)
>TREE_USED (scope) = false;
>if (!(*debug_hooks->ignore_block) (scope))
>  TREE_USED (scope) = true;
> +  for (t = BLOCK_VARS (scope); t; t = DECL_CHAIN (t))
> +if (VAR_P (t) && DECL_HAS_VALUE_EXPR_P (t))
> +  {
> +   tree dve = DECL_VALUE_EXPR (t);
> +   mark_all_vars_used (&dve);
> +  }
>for (t = BLOCK_SUBBLOCKS (scope); t ; t = BLOCK_CHAIN (t))
>  mark_scope_block_unused (t);
>  }
> @@ -792,9 +808,10 @@ remove_unused_locals (void)
> 
>timevar_push (TV_REMOVE_UNUSED);
> 
> +  usedvars = BITMAP_ALLOC (NULL);
> +
>mark_scope_block_unused (DECL_INITIAL (current_function_decl));
> 
> -  usedvars = BITMAP_ALLOC (NULL);
>auto_bitmap useddebug;
> 
>/* Walk the CFG marking all referenced symbols.  */
> @@ -819,7 +836,7 @@ remove_unused_locals (void)
> {
>   if (gimple_debug_bind_p (stmt))
> {
> - tree var = gimple_debug_bind_get_var  (stmt);
> + tree var = gimple_debug_bind_get_var (stmt);
>   if (VAR_P (var))
> {
>   if (!gimple_debug_bind_get_value (stmt))
> preserves it in local_decls where it was earlier removed.
> This keeps DECL_VALUE_EXPR (id_string.55) working until the expand pass.
> That throws the VAR_DECL from local_decls again in expand_used_vars, e.g.
> expand_one_var doesn't do anything for DECL_HAS_VALUE_EXPR_P VAR_DECLs.
> And then instantiate_decls even vec_frees the whole cfun->local_decls (though
> by that time DECL_VALUE_EXPR (id_string.55) has been garbage collected 
> already.
> So perhaps instead we want to move such vars from local_decls to BLOCK_VARS of
> the outermost block or something?  But how to find out if it wasn't there
> already?

I don't think moving is feasible, but ...

> Note, normally these VLA addresses don't have DECL_VALUE_EXPR and so are
> usually used in the IL.  It is just that tree-nested.cc in this case added
> DECL_VALUE_EXPR on those because they are (or were) referenced from nested
> function.

I think the intent for this is to reflect this into debug info.  So,
... I think tree-nested.cc should put amend the related VAR which should
be already somewhere in BLOCK_VARS?

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-12 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #28 from Jakub Jelinek  ---
--- gcc/tree-ssa-live.cc.jj 2025-02-12 11:03:53.607556187 +0100
+++ gcc/tree-ssa-live.cc2025-02-12 11:54:34.779131237 +0100
@@ -369,9 +369,17 @@ mark_all_vars_used_1 (tree *tp, int *wal
 {
   /* When a global var becomes used for the first time also walk its
  initializer (non global ones don't have any).  */
-  if (set_is_used (t) && is_global_var (t)
- && DECL_CONTEXT (t) == current_function_decl)
-   mark_all_vars_used (&DECL_INITIAL (t));
+  if (set_is_used (t))
+   {
+ if (is_global_var (t)
+ && DECL_CONTEXT (t) == current_function_decl)
+   mark_all_vars_used (&DECL_INITIAL (t));
+ if (DECL_HAS_VALUE_EXPR_P (t))
+   {
+ tree dve = DECL_VALUE_EXPR (t);
+ mark_all_vars_used (&dve);
+   }
+   }
 }
   /* remove_unused_scope_block_p requires information about labels
  which are not DECL_IGNORED_P to tell if they might be used in the IL.  */
@@ -389,7 +397,9 @@ mark_all_vars_used_1 (tree *tp, int *wal
 }

 /* Mark the scope block SCOPE and its subblocks unused when they can be
-   possibly eliminated if dead.  */
+   possibly eliminated if dead.  Also, as remove_unused_scope_block_p is going
+   to keep all BLOCK_VARS VAR_DECLs with DECL_HAS_VALUE_EXPR_P, mark all vars
+   their DECL_VALUE_EXPR refers to.  */

 static void
 mark_scope_block_unused (tree scope)
@@ -398,6 +408,12 @@ mark_scope_block_unused (tree scope)
   TREE_USED (scope) = false;
   if (!(*debug_hooks->ignore_block) (scope))
 TREE_USED (scope) = true;
+  for (t = BLOCK_VARS (scope); t; t = DECL_CHAIN (t))
+if (VAR_P (t) && DECL_HAS_VALUE_EXPR_P (t))
+  {
+   tree dve = DECL_VALUE_EXPR (t);
+   mark_all_vars_used (&dve);
+  }
   for (t = BLOCK_SUBBLOCKS (scope); t ; t = BLOCK_CHAIN (t))
 mark_scope_block_unused (t);
 }
@@ -792,9 +808,10 @@ remove_unused_locals (void)

   timevar_push (TV_REMOVE_UNUSED);

+  usedvars = BITMAP_ALLOC (NULL);
+
   mark_scope_block_unused (DECL_INITIAL (current_function_decl));

-  usedvars = BITMAP_ALLOC (NULL);
   auto_bitmap useddebug;

   /* Walk the CFG marking all referenced symbols.  */
@@ -819,7 +836,7 @@ remove_unused_locals (void)
{
  if (gimple_debug_bind_p (stmt))
{
- tree var = gimple_debug_bind_get_var  (stmt);
+ tree var = gimple_debug_bind_get_var (stmt);
  if (VAR_P (var))
{
  if (!gimple_debug_bind_get_value (stmt))
preserves it in local_decls where it was earlier removed.
This keeps DECL_VALUE_EXPR (id_string.55) working until the expand pass.
That throws the VAR_DECL from local_decls again in expand_used_vars, e.g.
expand_one_var doesn't do anything for DECL_HAS_VALUE_EXPR_P VAR_DECLs.
And then instantiate_decls even vec_frees the whole cfun->local_decls (though
by that time DECL_VALUE_EXPR (id_string.55) has been garbage collected already.
So perhaps instead we want to move such vars from local_decls to BLOCK_VARS of
the outermost block or something?  But how to find out if it wasn't there
already?

Note, normally these VLA addresses don't have DECL_VALUE_EXPR and so are
usually used in the IL.  It is just that tree-nested.cc in this case added
DECL_VALUE_EXPR on those because they are (or were) referenced from nested
function.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-12 Thread rguenther at suse dot de via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #27 from rguenther at suse dot de  ---
On Wed, 12 Feb 2025, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790
> 
> --- Comment #26 from Jakub Jelinek  ---
> So, seems id_string is mentioned in BLOCK_VARS of the ec_meminfo BLOCK:
> BLOCK #0 [written] 
>   SUPERCONTEXT: ec_meminfo
>   SUBBLOCKS: BLOCK #109 [written]  BLOCK #166 [written]  BLOCK #167 [written] 
> BLOCK #168 [written]  BLOCK #169 [written]  
>   VARS: rnsort check_error prt_data slash_proc i nproc error clpfx .id_string
> id_string line node k n18 smallpage nnuma j bucket hugepage memfree cached
> clstr coreids icoreid iorank ubound.30 irecv_status itag lastnode llfirst_time
> llnocomm maxth mpi_byte mpi_comm_world mpi_integer4 myproc myth rn i 
> even before any unused var removals.
> While id_string.55 is created in the gimplifier, gimplify_vla_decl on
> id_string:
> 1981  addr = create_tmp_var (ptr_type, get_name (decl));
> 1982  DECL_IGNORED_P (addr) = 0;
> 1983  t = build_fold_indirect_ref (addr);
> 1984  TREE_THIS_NOTRAP (t) = 1;
> 1985  SET_DECL_VALUE_EXPR (decl, t);
> and so it is added to cfun->local_decls.
> Now, during remove_unused_locals, neither id_string nor id_string.55 are
> actually marked as used, they are not used in the IL anymore.
> But id_string is preserved in BLOCK_VARS due to remove_unused_scope_block_p:
>   /* If a decl has a value expr, we need to instantiate it
>  regardless of debug info generation, to avoid codegen
>  differences in memory overlap tests.  update_equiv_regs() may
>  indirectly call validate_equiv_mem() to test whether a
>  SET_DEST overlaps with others, and if the value expr changes
>  by virtual register instantiation, we may get end up with
>  different results.  */
>   else if (VAR_P (*t) && DECL_HAS_VALUE_EXPR_P (*t))
> unused = false;
> (added in r0-93860).
> But nothing does that something similar for VAR_DECLs with
> DECL_HAS_VALUE_EXPR_P in local_decls.
> So, shall we always preserve all the DECL_HAS_VALUE_EXPR_P local_decls?

I think handling them consistently is important.  So either this or ...

> Or shall we mark the DECL_HAS_VALUE_EXPR_P VAR_DECLs and their 
> DECL_VALUE_EXPRs
> as used?

... this.  At this point (and for branches) I'd prefer to not change
behavior for BLOCK_VARs, so always preserving local_decls with
DECL_HAS_VALUE_EXPR_P is preferred (and least likely to cause
regressions?).

Note RTL expansion has its own idea on what decls are used, so I'm
not 100% sure what the effect is of another "unused" var in
local_decls but not BLOCK_VARs.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-12 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #26 from Jakub Jelinek  ---
So, seems id_string is mentioned in BLOCK_VARS of the ec_meminfo BLOCK:
BLOCK #0 [written] 
  SUPERCONTEXT: ec_meminfo
  SUBBLOCKS: BLOCK #109 [written]  BLOCK #166 [written]  BLOCK #167 [written] 
BLOCK #168 [written]  BLOCK #169 [written]  
  VARS: rnsort check_error prt_data slash_proc i nproc error clpfx .id_string
id_string line node k n18 smallpage nnuma j bucket hugepage memfree cached
clstr coreids icoreid iorank ubound.30 irecv_status itag lastnode llfirst_time
llnocomm maxth mpi_byte mpi_comm_world mpi_integer4 myproc myth rn i 
even before any unused var removals.
While id_string.55 is created in the gimplifier, gimplify_vla_decl on
id_string:
1981  addr = create_tmp_var (ptr_type, get_name (decl));
1982  DECL_IGNORED_P (addr) = 0;
1983  t = build_fold_indirect_ref (addr);
1984  TREE_THIS_NOTRAP (t) = 1;
1985  SET_DECL_VALUE_EXPR (decl, t);
and so it is added to cfun->local_decls.
Now, during remove_unused_locals, neither id_string nor id_string.55 are
actually marked as used, they are not used in the IL anymore.
But id_string is preserved in BLOCK_VARS due to remove_unused_scope_block_p:
  /* If a decl has a value expr, we need to instantiate it
 regardless of debug info generation, to avoid codegen
 differences in memory overlap tests.  update_equiv_regs() may
 indirectly call validate_equiv_mem() to test whether a
 SET_DEST overlaps with others, and if the value expr changes
 by virtual register instantiation, we may get end up with
 different results.  */
  else if (VAR_P (*t) && DECL_HAS_VALUE_EXPR_P (*t))
unused = false;
(added in r0-93860).
But nothing does that something similar for VAR_DECLs with
DECL_HAS_VALUE_EXPR_P in local_decls.
So, shall we always preserve all the DECL_HAS_VALUE_EXPR_P local_decls?
Or shall we mark the DECL_HAS_VALUE_EXPR_P VAR_DECLs and their DECL_VALUE_EXPRs
as used?
Something else?

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-11 Thread rguenth at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #25 from Richard Biener  ---
(In reply to Jakub Jelinek from comment #24)
> So
> --- gcc/tree-ssa-live.cc.jj   2025-01-02 11:23:05.915664859 +0100
> +++ gcc/tree-ssa-live.cc  2025-02-11 14:44:33.940178150 +0100
> @@ -369,9 +369,17 @@ mark_all_vars_used_1 (tree *tp, int *wal
>  {
>/* When a global var becomes used for the first time also walk its
>   initializer (non global ones don't have any).  */
> -  if (set_is_used (t) && is_global_var (t)
> -   && DECL_CONTEXT (t) == current_function_decl)
> - mark_all_vars_used (&DECL_INITIAL (t));
> +  if (set_is_used (t))
> + {
> +   if (is_global_var (t)
> +   && DECL_CONTEXT (t) == current_function_decl)
> + mark_all_vars_used (&DECL_INITIAL (t));
> +   if (DECL_HAS_VALUE_EXPR_P (t))
> + {
> +   tree dve = DECL_VALUE_EXPR (t);
> +   mark_all_vars_used (&dve);
> + }
> + }
>  }
>/* remove_unused_scope_block_p requires information about labels
>   which are not DECL_IGNORED_P to tell if they might be used in the IL. 
> */
> ?  Except this doesn't work, that new code isn't encountered on the
> testcase, so it must be something else that removes the vars.

Yes, sth like the above should work.  It might be also inlining not
copying over the vars - IIRC it also has some tricks, at least for
those in local_decls.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-11 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #24 from Jakub Jelinek  ---
So
--- gcc/tree-ssa-live.cc.jj 2025-01-02 11:23:05.915664859 +0100
+++ gcc/tree-ssa-live.cc2025-02-11 14:44:33.940178150 +0100
@@ -369,9 +369,17 @@ mark_all_vars_used_1 (tree *tp, int *wal
 {
   /* When a global var becomes used for the first time also walk its
  initializer (non global ones don't have any).  */
-  if (set_is_used (t) && is_global_var (t)
- && DECL_CONTEXT (t) == current_function_decl)
-   mark_all_vars_used (&DECL_INITIAL (t));
+  if (set_is_used (t))
+   {
+ if (is_global_var (t)
+ && DECL_CONTEXT (t) == current_function_decl)
+   mark_all_vars_used (&DECL_INITIAL (t));
+ if (DECL_HAS_VALUE_EXPR_P (t))
+   {
+ tree dve = DECL_VALUE_EXPR (t);
+ mark_all_vars_used (&dve);
+   }
+   }
 }
   /* remove_unused_scope_block_p requires information about labels
  which are not DECL_IGNORED_P to tell if they might be used in the IL.  */
?  Except this doesn't work, that new code isn't encountered on the testcase,
so it must be something else that removes the vars.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-11 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #23 from Jakub Jelinek  ---
(In reply to Richard Biener from comment #21)
> So the issue is not that we remove id_string.55 var from the hash but
> that somehow marking the id_string.55 using *id_string.55 faults then?
> Why?  Is that really *X with X DECL_VALUE_EXPR being id_string.55 again
> and X isn't marked before we process this hashtable?

We first see in the DECL_VALUE_EXPR hash map an entry for id_string.55 ->
FRAME_*.id_string.55, id_string.55 is not marked (nothing but DECL_VALUE_EXPR
of id_string refers to it), so we delete that entry, then later on in the hash
table
we find id_string -> *id_string.55 mapping, id_string is marked, so we mark
also
*id_string.55 and therefore keep that.  Except that we've deleted
DECL_VALUE_EXPR for it, so now id_string.55 has DECL_HAS_VALUE_EXPR_P but
DECL_VALUE_EXPR == NULL.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-11 Thread rguenth at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #22 from Richard Biener  ---
(In reply to Richard Biener from comment #21)
> I think the VAR_DECL should be reachable from elsewhere (BLOCK_VARS ideally,
> I'd like to get rid of local_decls).  IIRC I wondered at some point
> whether remove_unused_locals needs to walk DECL_VALUE_EXPRs but I think
> I went down an alternate place for the fix/PR I'm remembering (which now
> makes it difficult to track down...).

PR116907 exactly.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-11 Thread rguenth at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #21 from Richard Biener  ---
(In reply to Jakub Jelinek from comment #12)
> This looks like a GC bug.
> On the id_string.55 DECL_HAS_VALUE_EXPR_P bit is set and DECL_VALUE_EXPR is
> set to
> COMPONENT_REF during streaming in.
> But then
> #0  pointer_hash::mark_deleted (e=@0x7fffea128850: 0x1) at
> ../../gcc/hash-traits.h:204
> #1  0x0110d844 in hash_table xcallocator>::mark_deleted (v=@0x7fffea128850: 0x1) at
> ../../gcc/hash-table.h:552
> #2  0x0110b81c in hash_table xcallocator>::clear_slot (this=0x7fffea12a070, slot=0x7fffea128850) at
> ../../gcc/hash-table.h:969
> #3  0x01107f8c in gt_cleare_cache
> (h=0x7fffea12a070) at ../../gcc/hash-table.h:1315
> #4  0x01104afe in gt_clear_caches_gt_tree_h () at ./gt-tree.h:497
> #5  0x0044c95a in gt_clear_caches () at ./gtype-lto.h:1926
> #6  0x00771633 in ggc_mark_roots () at ../../gcc/ggc-common.cc:112
> #7  0x00490cb8 in ggc_collect (mode=GGC_COLLECT_HEURISTIC) at
> ../../gcc/ggc-page.cc:2231
> #8  0x00b0ce79 in execute_one_pass (pass= "expand"(277)>) at ../../gcc/passes.cc:2751
> marks the it in the value_expr_for_decl as deleted, because is ggc_marked_p
> false on it.
> But that very same id_string.55 var is then ggc_set_mark marked because
> there is
> id_string var with *id_string.55 DECL_VALUE_EXPR later in the
> value_expr_for_decl hash table.
> 
> gt_cleare_cache does only one pass through a hash_map:
>   for (typename table::iterator iter = h->begin (); iter != h->end ();
> ++iter)
> if (!table::is_empty (*iter) && !table::is_deleted (*iter))
>   {
> int res = H::keep_cache_entry (*iter);
> if (res == 0)
>   h->clear_slot (&*iter);
> else if (res != -1)
>   H::ggc_mx (*iter);
>   }

So the issue is not that we remove id_string.55 var from the hash but
that somehow marking the id_string.55 using *id_string.55 faults then?
Why?  Is that really *X with X DECL_VALUE_EXPR being id_string.55 again
and X isn't marked before we process this hashtable?

> So, either the VAR_DECL should be reachable from something other than
> DECL_VALUE_EXPR expressions (bet it was originally in local decls of
> something), or we need to have two passes over hash_maps, in the first pass
> just H::ggc_mx (*iter) on entries where res is not 0 nor -1, and in the
> second pass clear_slot of res is 0.
> Guess that needs to be done only for hash tables where keep_cache_entry
> returns 0/1 rather than 0/-1, though no idea how to that out during
> instantiation.
> And I wonder about references from multiple cache hash maps, say
> DECL_DEBUG_EXPR refers to something which has DECL_VALUE_EXPR or vice versa.
> Wouldn't we then need to do it always in two passes for all the cache hash
> tables,
> during proper marking walk all those hash tables and just H::ggc_mx (*iter)
> when keep_cache_entry returns 1, and then once everything is marked repeat
> the walks and only do the clear_slot?

I think the VAR_DECL should be reachable from elsewhere (BLOCK_VARS ideally,
I'd like to get rid of local_decls).  IIRC I wondered at some point
whether remove_unused_locals needs to walk DECL_VALUE_EXPRs but I think
I went down an alternate place for the fix/PR I'm remembering (which now
makes it difficult to track down...).

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-10 Thread sandra at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #20 from sandra at gcc dot gnu.org ---
Looks like other people are already investigating this?  I know nothing about
GC and this might not even have anything to do with the commit that caused the
regression to appear (like PR118714 that turned out to a stack corruption bug
in a previous commit).  But I can take a look if necessary to see if I can spot
something that doesn't look right.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-10 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

Jakub Jelinek  changed:

   What|Removed |Added

 CC||burnus at gcc dot gnu.org,
   ||sandra at gcc dot gnu.org

--- Comment #19 from Jakub Jelinek  ---
Note, the testcase doesn't use OpenMP declare variant nor metadirectives, so
wonder what exactly changed with that commit.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-10 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #18 from Jakub Jelinek  ---
For the reproduction it is important that the var mentioned in debug info with
*anothervar DECL_VALUE_EXPR value comes later in the DECL_VALUE_EXPR hash_map
than anothervar with FRAME_whatever.anothervar in DECL_VALUE_EXPR.  So that we
first
delete the hash map entry and then mark value whose entry has been deleted.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-10 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #17 from Jakub Jelinek  ---
No, it was r15-6895-g1294b819e1207c0ae76db29a75256f3fafd5f262

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-10 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #16 from Andrew Pinski  ---
I wonder if r15-4334-g60de5585812f59 exposed this issue.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-10 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #15 from Andrew Pinski  ---
Created attachment 60450
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=60450&action=edit
new testcase

`-O2 -fallow-argument-mismatch -g -fopenmp --param ggc-min-expand=0 --param
ggc-min-heapsize=0 -Wfatal-errors`

is enough to reproduce it now. Does not need LTO anymore.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-10 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

Andrew Pinski  changed:

   What|Removed |Added

   Keywords|lto |

--- Comment #14 from Andrew Pinski  ---
I am reducing this further now it seems like a GC issue.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-10 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #13 from Jakub Jelinek  ---
Note, the VAR_DECLs disappear from the local decls during ssa pass, but guess
id_string keeps to be referenced from debug info while id_string.55 only from
the DECL_VALUE_EXPR.
If DECL_VALUE_EXPR is the only table needing two passes (or if there are just a
couple), perhaps we might need some new GTY option to request two passes rather
than one for those.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-10 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

Jakub Jelinek  changed:

   What|Removed |Added

 CC||jakub at gcc dot gnu.org,
   ||rguenth at gcc dot gnu.org

--- Comment #12 from Jakub Jelinek  ---
This looks like a GC bug.
On the id_string.55 DECL_HAS_VALUE_EXPR_P bit is set and DECL_VALUE_EXPR is set
to
COMPONENT_REF during streaming in.
But then
#0  pointer_hash::mark_deleted (e=@0x7fffea128850: 0x1) at
../../gcc/hash-traits.h:204
#1  0x0110d844 in hash_table::mark_deleted (v=@0x7fffea128850: 0x1) at
../../gcc/hash-table.h:552
#2  0x0110b81c in hash_table::clear_slot (this=0x7fffea12a070, slot=0x7fffea128850) at
../../gcc/hash-table.h:969
#3  0x01107f8c in gt_cleare_cache
(h=0x7fffea12a070) at ../../gcc/hash-table.h:1315
#4  0x01104afe in gt_clear_caches_gt_tree_h () at ./gt-tree.h:497
#5  0x0044c95a in gt_clear_caches () at ./gtype-lto.h:1926
#6  0x00771633 in ggc_mark_roots () at ../../gcc/ggc-common.cc:112
#7  0x00490cb8 in ggc_collect (mode=GGC_COLLECT_HEURISTIC) at
../../gcc/ggc-page.cc:2231
#8  0x00b0ce79 in execute_one_pass (pass=) at ../../gcc/passes.cc:2751
marks the it in the value_expr_for_decl as deleted, because is ggc_marked_p
false on it.
But that very same id_string.55 var is then ggc_set_mark marked because there
is
id_string var with *id_string.55 DECL_VALUE_EXPR later in the
value_expr_for_decl hash table.

gt_cleare_cache does only one pass through a hash_map:
  for (typename table::iterator iter = h->begin (); iter != h->end (); ++iter)
if (!table::is_empty (*iter) && !table::is_deleted (*iter))
  {
int res = H::keep_cache_entry (*iter);
if (res == 0)
  h->clear_slot (&*iter);
else if (res != -1)
  H::ggc_mx (*iter);
  }
So, either the VAR_DECL should be reachable from something other than
DECL_VALUE_EXPR expressions (bet it was originally in local decls of
something), or we need to have two passes over hash_maps, in the first pass
just H::ggc_mx (*iter) on entries where res is not 0 nor -1, and in the second
pass clear_slot of res is 0.
Guess that needs to be done only for hash tables where keep_cache_entry returns
0/1 rather than 0/-1, though no idea how to that out during instantiation.
And I wonder about references from multiple cache hash maps, say
DECL_DEBUG_EXPR refers to something which has DECL_VALUE_EXPR or vice versa.
Wouldn't we then need to do it always in two passes for all the cache hash
tables,
during proper marking walk all those hash tables and just H::ggc_mx (*iter)
when keep_cache_entry returns 1, and then once everything is marked repeat the
walks and only do the clear_slot?

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-07 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #6 from Andrew Pinski  ---
Created attachment 60419
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=60419&action=edit
Semi reduced single file testcase

`gfortran  -O2 -flto=jobserver -shared -fallow-argument-mismatch -g  t.f90 
-fopenmp`

Still need LTO to get the ICE, have not looked into why though.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-07 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #5 from Andrew Pinski  ---
Reducing ...

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-07 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #4 from Andrew Pinski  ---
(In reply to Sam James from comment #3)
> Created attachment 60418 [details]
> gcc-118790.tar.xz
> 
> Here's a reproducer. I won't reduce it though.

This is just enough:
```
gfortran  -O2 -flto=jobserver -shared -fallow-argument-mismatch -g -fopenmp 
ec_args_mod.F90.fii ec_parkind.F90.fii mpl_mpif.F90.fii ec_meminfo.F90.fii

``` 
The order is important for the module creation.

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-07 Thread sjames at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

Sam James  changed:

   What|Removed |Added

 CC||sjames at gcc dot gnu.org

--- Comment #3 from Sam James  ---
Created attachment 60418
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=60418&action=edit
gcc-118790.tar.xz

Here's a reproducer. I won't reduce it though.

```
$ gfortran -c ec_args_mod.F90.fii
$ gfortran -c ec_parkind.F90.fii
$ gfortran -c mpl_mpif.F90.fii
$ gfortran -O2 -flto=jobserver -shared -fallow-argument-mismatch -g -fopenmp
ec_meminfo.F90.fii ec_args_mod.F90.fii ec_parkind.F90.fii mpl_mpif.F90.fii
[...]
/tmp/fiat/src/fiat/util/ec_meminfo.F90:485:23: internal compiler error:
Segmentation fault
[...]
```

[Bug debug/118790] [15 Regression] ICE when building fiat (crash in loc_list_from_tree_1)

2025-02-07 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118790

--- Comment #2 from Andrew Pinski  ---
case MEM_REF:
  if (!integer_zerop (TREE_OPERAND (loc, 1)))
{
  have_address = 1;
  goto do_plus;
}
  /* Fallthru.  */
case INDIRECT_REF:
  list_ret = loc_list_from_tree_1 (TREE_OPERAND (loc, 0), 0, context);
  have_address = 1;
  break;


And then the crash is at:
  switch (TREE_CODE (loc))

Which means there is a null for either MEM_REF/INDIRECT_REF