[PATCH] Fix ICE on view conversion between struct and integer

[PATCH] Fix ICE on view conversion between struct and integer

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 1903 bytes --]

Hi,

you can build a view conversion between pretty much anything in Ada including
between types with different sizes, although the compiler warns in this case
and gigi pads the smaller type to end up with the same size.

The attached testcase triggers an ICE at -O or above for one of them:

FAIL: gnat.dg/opt98.adb (test for excess errors)
Excess errors:
during GIMPLE pass: esra
+===========================GNAT BUG DETECTED==============================+
| 13.0.0 20220713 (experimental) [master 6f5cf9470aa] (x86_64-suse-linux) GCC error:|
| in gimplify_modify_expr, at gimplify.cc:6254                             |
| Error detected around /home/eric/cvs/gcc/gcc/testsuite/gnat.dg/opt98.adb:10:7|
| Compiling /home/eric/cvs/gcc/gcc/testsuite/gnat.dg/opt98.adb             |

  if (gimplify_ctxp->into_ssa && is_gimple_reg (*to_p))
    {
      /* We should have got an SSA name from the start.  */
      gcc_assert (TREE_CODE (*to_p) == SSA_NAME
		  || ! gimple_in_ssa_p (cfun));
    }

This happens from prepare_gimple_addressable for the variable to be marked with
DECL_NOT_GIMPLE_REG_P when its initialization is gimplified, so it's apparently
just a matter of setting the flag earlier.

Bootstrapped/regtested on x86-64/Linux, OK for the mainline?


2022-07-14  Eric Botcazou  <ebotcazou@adacore.com>

	* gimplify.cc (lookup_tmp_var): Add NOT_GIMPLE_REG boolean parameter
	and set DECL_NOT_GIMPLE_REG_P on the variable according to it.
	(internal_get_tmp_var): Add NOT_GIMPLE_REG boolean parameter and pass
	it in the call to lookup_tmp_var.
	(get_formal_tmp_var): Pass false in the call to lookup_tmp_var.
	(get_initialized_tmp_var): Likewise.
	(prepare_gimple_addressable): Call internal_get_tmp_var instead of
	get_initialized_tmp_var with NOT_GIMPLE_REG set to true.


2022-07-14  Eric Botcazou  <ebotcazou@adacore.com>

	* gnat.dg/opt98.ads, gnat.dg/opt98.adb: New test.

-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 3136 bytes --]

diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index 04990ad91a6..2ac7ca0855e 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -573,20 +573,26 @@ create_tmp_from_val (tree val)
 }
 
 /* Create a temporary to hold the value of VAL.  If IS_FORMAL, try to reuse
-   an existing expression temporary.  */
+   an existing expression temporary.  If NOT_GIMPLE_REG, mark it as such.  */
 
 static tree
-lookup_tmp_var (tree val, bool is_formal)
+lookup_tmp_var (tree val, bool is_formal, bool not_gimple_reg)
 {
   tree ret;
 
+  /* We cannot mark a formal temporary with DECL_NOT_GIMPLE_REG_P.  */
+  gcc_assert (!is_formal || !not_gimple_reg);
+
   /* If not optimizing, never really reuse a temporary.  local-alloc
      won't allocate any variable that is used in more than one basic
      block, which means it will go into memory, causing much extra
      work in reload and final and poorer code generation, outweighing
      the extra memory allocation here.  */
   if (!optimize || !is_formal || TREE_SIDE_EFFECTS (val))
-    ret = create_tmp_from_val (val);
+    {
+      ret = create_tmp_from_val (val);
+      DECL_NOT_GIMPLE_REG_P (ret) = not_gimple_reg;
+    }
   else
     {
       elt_t elt, *elt_p;
@@ -617,7 +623,7 @@ lookup_tmp_var (tree val, bool is_formal)
 
 static tree
 internal_get_tmp_var (tree val, gimple_seq *pre_p, gimple_seq *post_p,
-                      bool is_formal, bool allow_ssa)
+		      bool is_formal, bool allow_ssa, bool not_gimple_reg)
 {
   tree t, mod;
 
@@ -639,7 +645,7 @@ internal_get_tmp_var (tree val, gimple_seq *pre_p, gimple_seq *post_p,
 	}
     }
   else
-    t = lookup_tmp_var (val, is_formal);
+    t = lookup_tmp_var (val, is_formal, not_gimple_reg);
 
   mod = build2 (INIT_EXPR, TREE_TYPE (t), t, unshare_expr (val));
 
@@ -667,7 +673,7 @@ internal_get_tmp_var (tree val, gimple_seq *pre_p, gimple_seq *post_p,
 tree
 get_formal_tmp_var (tree val, gimple_seq *pre_p)
 {
-  return internal_get_tmp_var (val, pre_p, NULL, true, true);
+  return internal_get_tmp_var (val, pre_p, NULL, true, true, false);
 }
 
 /* Return a temporary variable initialized with VAL.  PRE_P and POST_P
@@ -678,7 +684,7 @@ get_initialized_tmp_var (tree val, gimple_seq *pre_p,
 			 gimple_seq *post_p /* = NULL */,
 			 bool allow_ssa /* = true */)
 {
-  return internal_get_tmp_var (val, pre_p, post_p, false, allow_ssa);
+  return internal_get_tmp_var (val, pre_p, post_p, false, allow_ssa, false);
 }
 
 /* Declare all the variables in VARS in SCOPE.  If DEBUG_INFO is true,
@@ -4574,13 +4580,10 @@ prepare_gimple_addressable (tree *expr_p, gimple_seq *seq_p)
 {
   while (handled_component_p (*expr_p))
     expr_p = &TREE_OPERAND (*expr_p, 0);
+
+  /* Do not allow an SSA name as the temporary.  */
   if (is_gimple_reg (*expr_p))
-    {
-      /* Do not allow an SSA name as the temporary.  */
-      tree var = get_initialized_tmp_var (*expr_p, seq_p, NULL, false);
-      DECL_NOT_GIMPLE_REG_P (var) = 1;
-      *expr_p = var;
-    }
+    *expr_p = internal_get_tmp_var (*expr_p, seq_p, NULL, false, false, true);
 }
 
 /* A subroutine of gimplify_modify_expr.  Replace a MODIFY_EXPR with

[-- Attachment #3: opt98.ads --]
[-- Type: text/x-adasrc, Size: 344 bytes --]

with Ada.Unchecked_Conversion;
with System;

package Opt98 is

  type Rec is record
    I : Integer;
  end record;

  function To_Address is new Ada.Unchecked_Conversion (Rec, System.Address);

  function To_Rec is new Ada.Unchecked_Conversion (System.Address, Rec);

  A : System.Address with Atomic;

  function Func return Rec;

end Opt98;


[-- Attachment #4: opt98.adb --]
[-- Type: text/x-adasrc, Size: 212 bytes --]

-- { dg-do compile }
-- { dg-options "-O -gnatws" }

package body Opt98 is

  function Func return Rec is
    R :Rec;
  begin
    A := To_Address ((I => 0));
    R := To_Rec (A);
    return R;
  end;

end Opt98;

Re: [PATCH] Fix ICE on view conversion between struct and integer

[Richard Biener]

On Thu, Jul 14, 2022 at 10:20 AM Eric Botcazou via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi,
>
> you can build a view conversion between pretty much anything in Ada including
> between types with different sizes, although the compiler warns in this case
> and gigi pads the smaller type to end up with the same size.
>
> The attached testcase triggers an ICE at -O or above for one of them:
>
> FAIL: gnat.dg/opt98.adb (test for excess errors)
> Excess errors:
> during GIMPLE pass: esra
> +===========================GNAT BUG DETECTED==============================+
> | 13.0.0 20220713 (experimental) [master 6f5cf9470aa] (x86_64-suse-linux) GCC error:|
> | in gimplify_modify_expr, at gimplify.cc:6254                             |
> | Error detected around /home/eric/cvs/gcc/gcc/testsuite/gnat.dg/opt98.adb:10:7|
> | Compiling /home/eric/cvs/gcc/gcc/testsuite/gnat.dg/opt98.adb             |
>
>   if (gimplify_ctxp->into_ssa && is_gimple_reg (*to_p))
>     {
>       /* We should have got an SSA name from the start.  */
>       gcc_assert (TREE_CODE (*to_p) == SSA_NAME
>                   || ! gimple_in_ssa_p (cfun));
>     }
>
> This happens from prepare_gimple_addressable for the variable to be marked with
> DECL_NOT_GIMPLE_REG_P when its initialization is gimplified, so it's apparently
> just a matter of setting the flag earlier.
>
> Bootstrapped/regtested on x86-64/Linux, OK for the mainline?

OK.

Thanks,
Richard.

>
> 2022-07-14  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * gimplify.cc (lookup_tmp_var): Add NOT_GIMPLE_REG boolean parameter
>         and set DECL_NOT_GIMPLE_REG_P on the variable according to it.
>         (internal_get_tmp_var): Add NOT_GIMPLE_REG boolean parameter and pass
>         it in the call to lookup_tmp_var.
>         (get_formal_tmp_var): Pass false in the call to lookup_tmp_var.
>         (get_initialized_tmp_var): Likewise.
>         (prepare_gimple_addressable): Call internal_get_tmp_var instead of
>         get_initialized_tmp_var with NOT_GIMPLE_REG set to true.
>
>
> 2022-07-14  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * gnat.dg/opt98.ads, gnat.dg/opt98.adb: New test.
>
> --
> Eric Botcazou