public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PING] PR middle-end/95126: Expand small const structs as immediate constants
@ 2022-05-21 10:49 Roger Sayle
  2022-06-05 17:53 ` Andreas Schwab
  0 siblings, 1 reply; 11+ messages in thread
From: Roger Sayle @ 2022-05-21 10:49 UTC (permalink / raw)
  To: gcc-patches

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


I'd like to ping my patch for PR middle-end/95126 from February:
https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590949.html
As a regression fix, this has missed GCC 12, but hopefully its suitable
for 13 now we're back in stage1.

The patch has been refreshed and retested against gcc 13 trunk on
x86_64-pc-linux-gnu with make bootstrap and make -k check, both
with and without --target_board=unix{-m32} with no new failures.
Ok for mainline?

2022-05-21  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
	PR middle-end/95126
	* calls.cc (load_register_parameters): When loading a suitable
	immediate_const_ctor_p VAR_DECL into a single word_mode register,
	construct it directly in a pseudo rather than read it (by parts)
	from memory.
	* expr.cc (int_expr_size): Make tree argument a const_tree.
	(immediate_const_ctor_p): Helper predicate.  Return true for
	simple constructors that may be materialized in a register.
	(expand_expr_real_1) [VAR_DECL]: When expanding a constant
	VAR_DECL with a suitable immediate_const_ctor_p constructor
	use store_constructor to materialize it directly in a pseudo.
	* expr.h (immediate_const_ctor_p): Prototype here.
	* varasm.cc (initializer_constant_valid_for_bitfield_p): Change
	VALUE argument from tree to const_tree.
	* varasm.h (initializer_constant_valid_for_bitfield_p): Update
	prototype.

gcc/testsuite/ChangeLog
	PR middle-end/95126
	* gcc.target/i386/pr95126-m32-1.c: New test case.
	* gcc.target/i386/pr95126-m32-2.c: New test case.
	* gcc.target/i386/pr95126-m32-3.c: New test case.
	* gcc.target/i386/pr95126-m32-4.c: New test case.
	* gcc.target/i386/pr95126-m64-1.c: New test case.
	* gcc.target/i386/pr95126-m64-2.c: New test case.
	* gcc.target/i386/pr95126-m64-3.c: New test case.
	* gcc.target/i386/pr95126-m64-4.c: New test case.


Thanks in advance,
Roger
--


[-- Attachment #2: patchvd5b.txt --]
[-- Type: text/plain, Size: 12170 bytes --]

diff --git a/gcc/calls.cc b/gcc/calls.cc
index bbaf69c..a4336c1 100644
--- a/gcc/calls.cc
+++ b/gcc/calls.cc
@@ -2095,7 +2095,8 @@ load_register_parameters (struct arg_data *args, int num_actuals,
 	  poly_int64 size = 0;
 	  HOST_WIDE_INT const_size = 0;
 	  rtx_insn *before_arg = get_last_insn ();
-	  tree type = TREE_TYPE (args[i].tree_value);
+	  tree tree_value = args[i].tree_value;
+	  tree type = TREE_TYPE (tree_value);
 	  if (RECORD_OR_UNION_TYPE_P (type) && TYPE_TRANSPARENT_AGGR (type))
 	    type = TREE_TYPE (first_field (type));
 	  /* Set non-negative if we must move a word at a time, even if
@@ -2172,6 +2173,24 @@ load_register_parameters (struct arg_data *args, int num_actuals,
 	      emit_move_insn (gen_rtx_REG (word_mode, REGNO (reg) + j),
 			      args[i].aligned_regs[j]);
 
+	  /* If we need a single register and the source is a constant
+	     VAR_DECL with a simple constructor, expand that constructor
+	     via a pseudo rather than read from (possibly misaligned)
+	     memory.  PR middle-end/95126.  */
+	  else if (nregs == 1
+		   && partial == 0
+		   && !args[i].pass_on_stack
+		   && VAR_P (tree_value)
+		   && TREE_READONLY (tree_value)
+		   && !TREE_SIDE_EFFECTS (tree_value)
+		   && immediate_const_ctor_p (DECL_INITIAL (tree_value)))
+	    {
+	      rtx target = gen_reg_rtx (word_mode);
+	      rtx x = expand_expr (DECL_INITIAL (tree_value),
+				   target, word_mode, EXPAND_NORMAL);
+	      reg = gen_rtx_REG (word_mode, REGNO (reg));
+	      emit_move_insn (reg, x);
+	    }
 	  else if (partial == 0 || args[i].pass_on_stack)
 	    {
 	      /* SIZE and CONST_SIZE are 0 for partial arguments and
diff --git a/gcc/expr.cc b/gcc/expr.cc
index 1806091..26f5528 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -100,7 +100,7 @@ static void do_tablejump (rtx, machine_mode, rtx, rtx, rtx,
 			  profile_probability);
 static rtx const_vector_from_tree (tree);
 static tree tree_expr_size (const_tree);
-static HOST_WIDE_INT int_expr_size (tree);
+static HOST_WIDE_INT int_expr_size (const_tree);
 static void convert_mode_scalar (rtx, rtx, int);
 
 \f
@@ -4867,7 +4867,22 @@ emit_push_insn (rtx x, machine_mode mode, tree type, rtx size,
 		    return false;
 		}
 	    }
-	  emit_block_move (target, xinner, size, BLOCK_OP_CALL_PARM);
+
+	  /* If source is a constant VAR_DECL with a simple constructor,
+             store the constructor to the stack instead of moving it.  */
+	  const_tree decl;
+	  if (partial == 0
+	      && MEM_P (xinner)
+	      && SYMBOL_REF_P (XEXP (xinner, 0))
+	      && (decl = SYMBOL_REF_DECL (XEXP (xinner, 0))) != NULL_TREE
+	      && VAR_P (decl)
+	      && TREE_READONLY (decl)
+	      && !TREE_SIDE_EFFECTS (decl)
+	      && immediate_const_ctor_p (DECL_INITIAL (decl), 2))
+	    store_constructor (DECL_INITIAL (decl), target, 0,
+			       int_expr_size (DECL_INITIAL (decl)), false);
+	  else
+	    emit_block_move (target, xinner, size, BLOCK_OP_CALL_PARM);
 	}
     }
   else if (partial > 0)
@@ -6576,6 +6591,25 @@ categorize_ctor_elements (const_tree ctor, HOST_WIDE_INT *p_nz_elts,
 				     p_init_elts, p_complete);
 }
 
+/* Return true if constructor CTOR is simple enough to be materialized
+   in an integer mode register.  Limit the size to WORDS words, which
+   is 1 by default.  */
+
+bool
+immediate_const_ctor_p (const_tree ctor, unsigned int words)
+{
+  /* Allow function to be called with a VAR_DECL's DECL_INITIAL.  */
+  if (!ctor || TREE_CODE (ctor) != CONSTRUCTOR)
+    return false;
+
+  return TREE_CONSTANT (ctor)
+	 && !TREE_ADDRESSABLE (ctor)
+	 && CONSTRUCTOR_NELTS (ctor)
+	 && TREE_CODE (TREE_TYPE (ctor)) != ARRAY_TYPE
+	 && int_expr_size (ctor) <= words * UNITS_PER_WORD
+	 && initializer_constant_valid_for_bitfield_p (ctor);
+}
+
 /* TYPE is initialized by a constructor with NUM_ELTS elements, the last
    of which had type LAST_TYPE.  Each element was itself a complete
    initializer, in the sense that every meaningful byte was explicitly
@@ -10535,6 +10569,21 @@ expand_expr_real_1 (tree exp, rtx target, machine_mode tmode,
 	  if (temp)
 	    return temp;
 	}
+      /* Expand const VAR_DECLs with CONSTRUCTOR initializers that
+	 have scalar integer modes to a reg via store_constructor.  */
+      if (TREE_READONLY (exp)
+	  && !TREE_SIDE_EFFECTS (exp)
+	  && (modifier == EXPAND_NORMAL || modifier == EXPAND_STACK_PARM)
+	  && immediate_const_ctor_p (DECL_INITIAL (exp))
+	  && SCALAR_INT_MODE_P (TYPE_MODE (TREE_TYPE (exp)))
+	  && crtl->emit.regno_pointer_align_length
+	  && !target)
+	{
+	  target = gen_reg_rtx (TYPE_MODE (TREE_TYPE (exp)));
+	  store_constructor (DECL_INITIAL (exp), target, 0,
+			     int_expr_size (DECL_INITIAL (exp)), false);
+	  return target;
+	}
       /* ... fall through ...  */
 
     case PARM_DECL:
@@ -13129,7 +13178,7 @@ expr_size (tree exp)
    if the size can vary or is larger than an integer.  */
 
 static HOST_WIDE_INT
-int_expr_size (tree exp)
+int_expr_size (const_tree exp)
 {
   tree size;
 
diff --git a/gcc/expr.h b/gcc/expr.h
index 7e5cf49..d777c28 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -338,6 +338,7 @@ extern unsigned HOST_WIDE_INT highest_pow2_factor (const_tree);
 extern bool categorize_ctor_elements (const_tree, HOST_WIDE_INT *,
 				      HOST_WIDE_INT *, HOST_WIDE_INT *,
 				      bool *);
+extern bool immediate_const_ctor_p (const_tree, unsigned int words = 1);
 
 extern void expand_operands (tree, tree, rtx, rtx*, rtx*,
 			     enum expand_modifier);
diff --git a/gcc/varasm.cc b/gcc/varasm.cc
index 6454f1c..826a9ca 100644
--- a/gcc/varasm.cc
+++ b/gcc/varasm.cc
@@ -5069,7 +5069,7 @@ initializer_constant_valid_p (tree value, tree endtype, bool reverse)
    an element of a "constant" initializer.  */
 
 bool
-initializer_constant_valid_for_bitfield_p (tree value)
+initializer_constant_valid_for_bitfield_p (const_tree value)
 {
   /* For bitfields we support integer constants or possibly nested aggregates
      of such.  */
@@ -5078,7 +5078,7 @@ initializer_constant_valid_for_bitfield_p (tree value)
     case CONSTRUCTOR:
       {
 	unsigned HOST_WIDE_INT idx;
-	tree elt;
+	const_tree elt;
 
 	FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (value), idx, elt)
 	  if (!initializer_constant_valid_for_bitfield_p (elt))
diff --git a/gcc/varasm.h b/gcc/varasm.h
index 8ba8374..acbd9fa 100644
--- a/gcc/varasm.h
+++ b/gcc/varasm.h
@@ -65,7 +65,7 @@ extern tree initializer_constant_valid_p (tree, tree, bool = false);
 /* Return true if VALUE is a valid constant-valued expression
    for use in initializing a static bit-field; one that can be
    an element of a "constant" initializer.  */
-extern bool initializer_constant_valid_for_bitfield_p (tree);
+extern bool initializer_constant_valid_for_bitfield_p (const_tree);
 
 /* Whether a constructor CTOR is a valid static constant initializer if all
    its elements are.  This used to be internal to initializer_constant_valid_p
diff --git a/gcc/testsuite/gcc.target/i386/pr95126-m32-1.c b/gcc/testsuite/gcc.target/i386/pr95126-m32-1.c
new file mode 100644
index 0000000..1d6acd6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr95126-m32-1.c
@@ -0,0 +1,16 @@
+/* { dg-do compile { target { ia32 } } } */
+/* { dg-options "-O2" } */
+
+struct small{ short a,b; signed char c; };
+
+void call_func(void)
+{
+    extern int func(struct small X);
+    static struct small const s = { 1,2,0 };
+    func(s);
+}
+
+/* { dg-final { scan-assembler "movl\[ \\t]*\\\$" } } */
+/* { dg-final { scan-assembler "movb\[ \\t]*\\\$0, " } } */
+/* { dg-final { scan-assembler-not "movzwl" } } */
+
diff --git a/gcc/testsuite/gcc.target/i386/pr95126-m32-2.c b/gcc/testsuite/gcc.target/i386/pr95126-m32-2.c
new file mode 100644
index 0000000..b46be9d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr95126-m32-2.c
@@ -0,0 +1,16 @@
+/* { dg-do compile { target { ia32 } } } */
+/* { dg-options "-O2" } */
+
+struct small{ short a,b; signed char c; };
+static const struct small s = { 1,2,0 };
+extern int func(struct small X);
+
+void call_func(void)
+{
+  func(s);
+}
+
+/* { dg-final { scan-assembler "movl\[ \\t]*\\\$" } } */
+/* { dg-final { scan-assembler "movb\[ \\t]*\\\$0, " } } */
+/* { dg-final { scan-assembler-not "movzwl" } } */
+
diff --git a/gcc/testsuite/gcc.target/i386/pr95126-m32-3.c b/gcc/testsuite/gcc.target/i386/pr95126-m32-3.c
new file mode 100644
index 0000000..cc2fe94
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr95126-m32-3.c
@@ -0,0 +1,15 @@
+/* { dg-do compile { target { ia32 } } } */
+/* { dg-options "-O2" } */
+
+struct small{ short a; };
+
+void call_func(void)
+{
+    extern int func(struct small X);
+    static struct small const s = { 2 };
+    func(s);
+}
+
+/* { dg-final { scan-assembler "pushl\[ \\t]*\\\$2" } } */
+/* { dg-final { scan-assembler-not "movzwl" } } */
+
diff --git a/gcc/testsuite/gcc.target/i386/pr95126-m32-4.c b/gcc/testsuite/gcc.target/i386/pr95126-m32-4.c
new file mode 100644
index 0000000..e829335
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr95126-m32-4.c
@@ -0,0 +1,13 @@
+/* { dg-do compile { target { ia32 } } } */
+/* { dg-options "-O2" } */
+
+struct small{ short a,b; };
+
+void call_func(void)
+{
+    extern int func(struct small X);
+    static struct small const s = { 1,2 };
+    func(s);
+}
+
+/* { dg-final { scan-assembler "pushl\[ \\t]*\\\$131073" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr95126-m64-1.c b/gcc/testsuite/gcc.target/i386/pr95126-m64-1.c
new file mode 100644
index 0000000..d5c6dded
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr95126-m64-1.c
@@ -0,0 +1,17 @@
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-O2" } */
+
+struct small{ short a,b; signed char c; };
+
+void call_func(void)
+{
+    extern int func(struct small X);
+    static struct small const s = { 1,2,0 };
+    func(s);
+}
+
+/* { dg-final { scan-assembler "movl\[ \\t]*\\\$131073, " } } */
+/* { dg-final { scan-assembler-not "movzwl" } } */
+/* { dg-final { scan-assembler-not "salq" } } */
+/* { dg-final { scan-assembler-not "orq" } } */
+
diff --git a/gcc/testsuite/gcc.target/i386/pr95126-m64-2.c b/gcc/testsuite/gcc.target/i386/pr95126-m64-2.c
new file mode 100644
index 0000000..0230ffc
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr95126-m64-2.c
@@ -0,0 +1,17 @@
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-O2" } */
+
+struct small{ short a,b; signed char c; };
+static const struct small s = { 1,2,0 };
+extern int func(struct small X);
+
+void call_func(void)
+{
+  func(s);
+}
+
+/* { dg-final { scan-assembler "movl\[ \\t]*\\\$131073, " } } */
+/* { dg-final { scan-assembler-not "movzwl" } } */
+/* { dg-final { scan-assembler-not "salq" } } */
+/* { dg-final { scan-assembler-not "orq" } } */
+
diff --git a/gcc/testsuite/gcc.target/i386/pr95126-m64-3.c b/gcc/testsuite/gcc.target/i386/pr95126-m64-3.c
new file mode 100644
index 0000000..25afe3a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr95126-m64-3.c
@@ -0,0 +1,14 @@
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-O2" } */
+
+struct small{ short a; };
+
+void call_func(void)
+{
+    extern int func(struct small X);
+    static struct small const s = { 2 };
+    func(s);
+}
+
+/* { dg-final { scan-assembler "movl\[ \\t]*\\\$2, " } } */
+/* { dg-final { scan-assembler-not "movzwl" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr95126-m64-4.c b/gcc/testsuite/gcc.target/i386/pr95126-m64-4.c
new file mode 100644
index 0000000..71c7908
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr95126-m64-4.c
@@ -0,0 +1,13 @@
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-O2" } */
+
+struct small{ short a,b; };
+
+void call_func(void)
+{
+    extern int func(struct small X);
+    static struct small const s = { 1,2 };
+    func(s);
+}
+
+/* { dg-final { scan-assembler "movl\[ \\t]*\\\$131073, " } } */

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PING] PR middle-end/95126: Expand small const structs as immediate constants
  2022-05-21 10:49 [PING] PR middle-end/95126: Expand small const structs as immediate constants Roger Sayle
@ 2022-06-05 17:53 ` Andreas Schwab
  2022-06-05 20:31   ` Rainer Orth
  0 siblings, 1 reply; 11+ messages in thread
From: Andreas Schwab @ 2022-06-05 17:53 UTC (permalink / raw)
  To: Roger Sayle; +Cc: gcc-patches

This breaks Ada on aarch64 in stage3, probably a miscompiled stage2
compiler.  For example:

/opt/gcc/gcc-20220605/Build/./prev-gcc/xgcc -B/opt/gcc/gcc-20220605/Build/./prev-gcc/ -B/usr/aarch64-suse-linux/bin/ -B/usr/aarch64-suse-linux/bin/ -B/usr/aarch64-suse-linux/lib/ -isystem /usr/aarch64-suse-linux/include -isystem /usr/aarch64-suse-linux/sys-include   -fchecking=1 -c -g -O2 -fchecking=1  -gnatpg -gnata -W -Wall -nostdinc -I- -I. -Iada/generated -Iada -I../../gcc/ada -Iada/libgnat -I../../gcc/ada/libgnat -Iada/gcc-interface -I../../gcc/ada/gcc-interface ../../gcc/ada/spark_xrefs.adb -o ada/spark_xrefs.o
+===========================GNAT BUG DETECTED==============================+
| 13.0.0 20220605 (experimental) [master ad6919374be] (aarch64-suse-linux) |
| Assert_Failure failed precondition from sinfo-nodes.ads:5419             |
| Error detected at types.ads:53:28                                        |
| Compiling ../../gcc/ada/spark_xrefs.adb                                  |
| Please submit a bug report; see https://gcc.gnu.org/bugs/ .              |
| Use a subject line meaningful to you and us to track the bug.            |
| Include the entire contents of this bug box in the report.               |
| Include the exact command that you entered.                              |
| Also include sources listed below.                                       |
+==========================================================================+

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PING] PR middle-end/95126: Expand small const structs as immediate constants
  2022-06-05 17:53 ` Andreas Schwab
@ 2022-06-05 20:31   ` Rainer Orth
  2022-06-06  8:43     ` Roger Sayle
  0 siblings, 1 reply; 11+ messages in thread
From: Rainer Orth @ 2022-06-05 20:31 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Roger Sayle, gcc-patches

Andreas Schwab <schwab@linux-m68k.org> writes:

> This breaks Ada on aarch64 in stage3, probably a miscompiled stage2
> compiler.  For example:
>
> /opt/gcc/gcc-20220605/Build/./prev-gcc/xgcc
> -B/opt/gcc/gcc-20220605/Build/./prev-gcc/ -B/usr/aarch64-suse-linux/bin/
> -B/usr/aarch64-suse-linux/bin/ -B/usr/aarch64-suse-linux/lib/ -isystem
> /usr/aarch64-suse-linux/include -isystem
> /usr/aarch64-suse-linux/sys-include -fchecking=1 -c -g -O2 -fchecking=1
> -gnatpg -gnata -W -Wall -nostdinc -I- -I. -Iada/generated -Iada
> -I../../gcc/ada -Iada/libgnat -I../../gcc/ada/libgnat -Iada/gcc-interface
> -I../../gcc/ada/gcc-interface ../../gcc/ada/spark_xrefs.adb -o
> ada/spark_xrefs.o
> +===========================GNAT BUG DETECTED==============================+
> | 13.0.0 20220605 (experimental) [master ad6919374be] (aarch64-suse-linux) |
> | Assert_Failure failed precondition from sinfo-nodes.ads:5419             |
> | Error detected at types.ads:53:28                                        |
> | Compiling ../../gcc/ada/spark_xrefs.adb                                  |
> | Please submit a bug report; see https://gcc.gnu.org/bugs/ .              |
> | Use a subject line meaningful to you and us to track the bug.            |
> | Include the entire contents of this bug box in the report.               |
> | Include the exact command that you entered.                              |
> | Also include sources listed below.                                       |
> +==========================================================================+

Confirmed: this also happens on i386-pc-solaris2.11,
sparc-sun-solaris2.11, and x86_64-pc-linux-gnu.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PING] PR middle-end/95126: Expand small const structs as immediate constants
  2022-06-05 20:31   ` Rainer Orth
@ 2022-06-06  8:43     ` Roger Sayle
  2022-06-06 11:16       ` Andreas Schwab
  2022-06-06 12:00       ` Rainer Orth
  0 siblings, 2 replies; 11+ messages in thread
From: Roger Sayle @ 2022-06-06  8:43 UTC (permalink / raw)
  To: 'Rainer Orth', 'Andreas Schwab'; +Cc: gcc-patches


Hi Rainer (and Andreas),
My sincere apologies for the breakage.  I'm currently regression testing a
solution to the ICEs introduced by my "small const struct" patch, which
fingers-crossed might also fix this Ada bootstrap.  For a while when
Rainer also reported the bootstrap failure is visible, on
x86_64-pc-linux-gnu,
I thought I'd be able to diagnose and fix the issue myself, but alas my
--enable-languages="all" builds (still) fail earlier with:

gcc -std=gnu99 -c -g  -gnatpg -gnatwns -gnata -W -Wall -I- -I.
-Iada/generated -Iada -I../../gcc/gcc/ada ../../gcc/gcc/ada/osint.adb -o
ada/osint.o
osint.adb:438:31: "strlen" not declared in "CRTL"
osint.adb:441:14: "strncpy" not declared in "CRTL"
osint.adb:675:21: "strlen" not declared in "CRTL"
osint.adb:728:16: "Open_Append" is undefined
osint.adb:1108:41: "int64" not declared in "CRTL"
osint.adb:3144:28: "strlen" not declared in "CRTL"
osint.adb:3147:11: "strncpy" not declared in "CRTL"

The one experiment I'd like to be able to try, to investigate the cause/cure
of this, is:

diff --git a/gcc/calls.cc b/gcc/calls.cc
index a4336c1..05fdd24 100644
--- a/gcc/calls.cc
+++ b/gcc/calls.cc
@@ -2177,7 +2177,7 @@ load_register_parameters (struct arg_data *args, int
num_a
             VAR_DECL with a simple constructor, expand that constructor
             via a pseudo rather than read from (possibly misaligned)
             memory.  PR middle-end/95126.  */
-         else if (nregs == 1
+         else if (0 && nregs == 1
                   && partial == 0
                   && !args[i].pass_on_stack
                   && VAR_P (tree_value)

My "small const structs" patch affected code generation in three places, and
identifying
which of these is causing the miscompilation issue for gnat will help narrow
down the
problem, or worst case allow reverting less of the problematic patch.

Again my deep apologies for the breakage.  Hopefully the fix I'm testing
will cure this as
well (but an ICE is different symptom to a silent miscompilation).

Sorry again,
Roger
--

> -----Original Message-----
> From: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
> Sent: 05 June 2022 21:31
> To: Andreas Schwab <schwab@linux-m68k.org>
> Cc: Roger Sayle <roger@nextmovesoftware.com>; gcc-patches@gcc.gnu.org
> Subject: Re: [PING] PR middle-end/95126: Expand small const structs as
> immediate constants
> 
> Andreas Schwab <schwab@linux-m68k.org> writes:
> 
> > This breaks Ada on aarch64 in stage3, probably a miscompiled stage2
> > compiler.  For example:
> >
> > /opt/gcc/gcc-20220605/Build/./prev-gcc/xgcc
> > -B/opt/gcc/gcc-20220605/Build/./prev-gcc/
> > -B/usr/aarch64-suse-linux/bin/ -B/usr/aarch64-suse-linux/bin/
> > -B/usr/aarch64-suse-linux/lib/ -isystem
> > /usr/aarch64-suse-linux/include -isystem
> > /usr/aarch64-suse-linux/sys-include -fchecking=1 -c -g -O2
> > -fchecking=1 -gnatpg -gnata -W -Wall -nostdinc -I- -I. -Iada/generated
> > -Iada -I../../gcc/ada -Iada/libgnat -I../../gcc/ada/libgnat
> > -Iada/gcc-interface -I../../gcc/ada/gcc-interface
> > ../../gcc/ada/spark_xrefs.adb -o ada/spark_xrefs.o
> > +===========================GNAT BUG
> > +DETECTED==============================+
> > | 13.0.0 20220605 (experimental) [master ad6919374be]
(aarch64-suse-linux)
> |
> > | Assert_Failure failed precondition from sinfo-nodes.ads:5419
|
> > | Error detected at types.ads:53:28
|
> > | Compiling ../../gcc/ada/spark_xrefs.adb
|
> > | Please submit a bug report; see https://gcc.gnu.org/bugs/ .
|
> > | Use a subject line meaningful to you and us to track the bug.
|
> > | Include the entire contents of this bug box in the report.
|
> > | Include the exact command that you entered.
|
> > | Also include sources listed below.
|
> >
> +================================================================
> =====
> > +=====+
> 
> Confirmed: this also happens on i386-pc-solaris2.11,
sparc-sun-solaris2.11, and
> x86_64-pc-linux-gnu.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PING] PR middle-end/95126: Expand small const structs as immediate constants
  2022-06-06  8:43     ` Roger Sayle
@ 2022-06-06 11:16       ` Andreas Schwab
  2022-06-06 11:23         ` Roger Sayle
  2022-06-06 12:01         ` Rainer Orth
  2022-06-06 12:00       ` Rainer Orth
  1 sibling, 2 replies; 11+ messages in thread
From: Andreas Schwab @ 2022-06-06 11:16 UTC (permalink / raw)
  To: Roger Sayle; +Cc: 'Rainer Orth', gcc-patches

On Jun 06 2022, Roger Sayle wrote:

> gcc -std=gnu99 -c -g  -gnatpg -gnatwns -gnata -W -Wall -I- -I.
> -Iada/generated -Iada -I../../gcc/gcc/ada ../../gcc/gcc/ada/osint.adb -o
> ada/osint.o
> osint.adb:438:31: "strlen" not declared in "CRTL"
> osint.adb:441:14: "strncpy" not declared in "CRTL"
> osint.adb:675:21: "strlen" not declared in "CRTL"
> osint.adb:728:16: "Open_Append" is undefined
> osint.adb:1108:41: "int64" not declared in "CRTL"
> osint.adb:3144:28: "strlen" not declared in "CRTL"
> osint.adb:3147:11: "strncpy" not declared in "CRTL"

What's your bootstrap compiler?

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PING] PR middle-end/95126: Expand small const structs as immediate constants
  2022-06-06 11:16       ` Andreas Schwab
@ 2022-06-06 11:23         ` Roger Sayle
  2022-06-06 11:44           ` Andreas Schwab
  2022-06-06 12:01         ` Rainer Orth
  1 sibling, 1 reply; 11+ messages in thread
From: Roger Sayle @ 2022-06-06 11:23 UTC (permalink / raw)
  To: 'Andreas Schwab'; +Cc: gcc-patches


Hi Andreas, 
> > gcc -std=gnu99 -c -g  -gnatpg -gnatwns -gnata -W -Wall -I- -I.
> > -Iada/generated -Iada -I../../gcc/gcc/ada ../../gcc/gcc/ada/osint.adb
> > -o ada/osint.o
> > osint.adb:438:31: "strlen" not declared in "CRTL"
> > osint.adb:441:14: "strncpy" not declared in "CRTL"
> > osint.adb:675:21: "strlen" not declared in "CRTL"
> > osint.adb:728:16: "Open_Append" is undefined
> > osint.adb:1108:41: "int64" not declared in "CRTL"
> > osint.adb:3144:28: "strlen" not declared in "CRTL"
> > osint.adb:3147:11: "strncpy" not declared in "CRTL"
> 
> What's your bootstrap compiler?

GNAT 4.8.5 20150623 (Red Hat 4.8.5-44)

I'm currently investigating other options...



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PING] PR middle-end/95126: Expand small const structs as immediate constants
  2022-06-06 11:23         ` Roger Sayle
@ 2022-06-06 11:44           ` Andreas Schwab
  0 siblings, 0 replies; 11+ messages in thread
From: Andreas Schwab @ 2022-06-06 11:44 UTC (permalink / raw)
  To: Roger Sayle; +Cc: gcc-patches

On Jun 06 2022, Roger Sayle wrote:

> Hi Andreas, 
>> > gcc -std=gnu99 -c -g  -gnatpg -gnatwns -gnata -W -Wall -I- -I.
>> > -Iada/generated -Iada -I../../gcc/gcc/ada ../../gcc/gcc/ada/osint.adb
>> > -o ada/osint.o
>> > osint.adb:438:31: "strlen" not declared in "CRTL"
>> > osint.adb:441:14: "strncpy" not declared in "CRTL"
>> > osint.adb:675:21: "strlen" not declared in "CRTL"
>> > osint.adb:728:16: "Open_Append" is undefined
>> > osint.adb:1108:41: "int64" not declared in "CRTL"
>> > osint.adb:3144:28: "strlen" not declared in "CRTL"
>> > osint.adb:3147:11: "strncpy" not declared in "CRTL"
>> 
>> What's your bootstrap compiler?
>
> GNAT 4.8.5 20150623 (Red Hat 4.8.5-44)

That's very much out of date.

    In order to build GNAT, the Ada compiler, you need a working GNAT
    compiler (GCC version 5.1 or later).

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PING] PR middle-end/95126: Expand small const structs as immediate constants
  2022-06-06  8:43     ` Roger Sayle
  2022-06-06 11:16       ` Andreas Schwab
@ 2022-06-06 12:00       ` Rainer Orth
  2022-06-06 12:29         ` Roger Sayle
  1 sibling, 1 reply; 11+ messages in thread
From: Rainer Orth @ 2022-06-06 12:00 UTC (permalink / raw)
  To: Roger Sayle; +Cc: 'Andreas Schwab', gcc-patches

Hi Roger,

> My sincere apologies for the breakage.  I'm currently regression testing a
> solution to the ICEs introduced by my "small const struct" patch, which
> fingers-crossed might also fix this Ada bootstrap.  For a while when
> Rainer also reported the bootstrap failure is visible, on
> x86_64-pc-linux-gnu,
> I thought I'd be able to diagnose and fix the issue myself, but alas my
> --enable-languages="all" builds (still) fail earlier with:
>
> gcc -std=gnu99 -c -g  -gnatpg -gnatwns -gnata -W -Wall -I- -I.
> -Iada/generated -Iada -I../../gcc/gcc/ada ../../gcc/gcc/ada/osint.adb -o
> ada/osint.o
> osint.adb:438:31: "strlen" not declared in "CRTL"
> osint.adb:441:14: "strncpy" not declared in "CRTL"
> osint.adb:675:21: "strlen" not declared in "CRTL"
> osint.adb:728:16: "Open_Append" is undefined
> osint.adb:1108:41: "int64" not declared in "CRTL"
> osint.adb:3144:28: "strlen" not declared in "CRTL"
> osint.adb:3147:11: "strncpy" not declared in "CRTL"
>
> The one experiment I'd like to be able to try, to investigate the cause/cure
> of this, is:
>
> diff --git a/gcc/calls.cc b/gcc/calls.cc
> index a4336c1..05fdd24 100644
> --- a/gcc/calls.cc
> +++ b/gcc/calls.cc
> @@ -2177,7 +2177,7 @@ load_register_parameters (struct arg_data *args, int
> num_a
>              VAR_DECL with a simple constructor, expand that constructor
>              via a pseudo rather than read from (possibly misaligned)
>              memory.  PR middle-end/95126.  */
> -         else if (nregs == 1
> +         else if (0 && nregs == 1
>                    && partial == 0
>                    && !args[i].pass_on_stack
>                    && VAR_P (tree_value)
>
> My "small const structs" patch affected code generation in three places, and
> identifying
> which of these is causing the miscompilation issue for gnat will help narrow
> down the
> problem, or worst case allow reverting less of the problematic patch.

I just tried this on i386-pc-solaris2.11: unfortunately, it made no
difference.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PING] PR middle-end/95126: Expand small const structs as immediate constants
  2022-06-06 11:16       ` Andreas Schwab
  2022-06-06 11:23         ` Roger Sayle
@ 2022-06-06 12:01         ` Rainer Orth
  1 sibling, 0 replies; 11+ messages in thread
From: Rainer Orth @ 2022-06-06 12:01 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Roger Sayle, gcc-patches

Andreas Schwab <schwab@linux-m68k.org> writes:

> On Jun 06 2022, Roger Sayle wrote:
>
>> gcc -std=gnu99 -c -g  -gnatpg -gnatwns -gnata -W -Wall -I- -I.
>> -Iada/generated -Iada -I../../gcc/gcc/ada ../../gcc/gcc/ada/osint.adb -o
>> ada/osint.o
>> osint.adb:438:31: "strlen" not declared in "CRTL"
>> osint.adb:441:14: "strncpy" not declared in "CRTL"
>> osint.adb:675:21: "strlen" not declared in "CRTL"
>> osint.adb:728:16: "Open_Append" is undefined
>> osint.adb:1108:41: "int64" not declared in "CRTL"
>> osint.adb:3144:28: "strlen" not declared in "CRTL"
>> osint.adb:3147:11: "strncpy" not declared in "CRTL"
>
> What's your bootstrap compiler?

FWIW, I'm using GCC 9 (9.4.0 on Solaris/x86, 9.3.0 on Linux/x86_64) as
bootstrap compiler.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PING] PR middle-end/95126: Expand small const structs as immediate constants
  2022-06-06 12:00       ` Rainer Orth
@ 2022-06-06 12:29         ` Roger Sayle
  2022-06-06 15:52           ` Rainer Orth
  0 siblings, 1 reply; 11+ messages in thread
From: Roger Sayle @ 2022-06-06 12:29 UTC (permalink / raw)
  To: 'Rainer Orth'; +Cc: 'Andreas Schwab', gcc-patches


Hi Rainer,

> > The one experiment I'd like to be able to try, to investigate the
> > cause/cure of this, is:
> >
> > diff --git a/gcc/calls.cc b/gcc/calls.cc index a4336c1..05fdd24 100644
> > --- a/gcc/calls.cc
> > +++ b/gcc/calls.cc
> > @@ -2177,7 +2177,7 @@ load_register_parameters (struct arg_data *args,
> > int num_a
> >              VAR_DECL with a simple constructor, expand that constructor
> >              via a pseudo rather than read from (possibly misaligned)
> >              memory.  PR middle-end/95126.  */
> > -         else if (nregs == 1
> > +         else if (0 && nregs == 1
> >                    && partial == 0
> >                    && !args[i].pass_on_stack
> >                    && VAR_P (tree_value)
> >
> > My "small const structs" patch affected code generation in three
> > places, and identifying which of these is causing the miscompilation
> > issue for gnat will help narrow down the problem, or worst case allow
> > reverting less of the problematic patch.
> 
> I just tried this on i386-pc-solaris2.11: unfortunately, it made no
difference.

Awesome!  Very many thanks for trying this.  Alas it confirms that the patch
I'm currently spinning won't have any affect.  So by a process of
elimination,
the miscompilation must be triggered by one of the other two changes:

diff --git a/gcc/expr.cc b/gcc/expr.cc
index fb062dc..37f1405 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -4871,7 +4871,7 @@ emit_push_insn (rtx x, machine_mode mode, tree type,
rtx s
          /* If source is a constant VAR_DECL with a simple constructor,
              store the constructor to the stack instead of moving it.  */
          const_tree decl;
-         if (partial == 0
+         if (0 && partial == 0
              && MEM_P (xinner)
              && SYMBOL_REF_P (XEXP (xinner, 0))
              && (decl = SYMBOL_REF_DECL (XEXP (xinner, 0))) != NULL_TREE
@@ -10603,7 +10603,7 @@ expand_expr_real_1 (tree exp, rtx target,
machine_mode t
        }
       /* Expand const VAR_DECLs with CONSTRUCTOR initializers that
         have scalar integer modes to a reg via store_constructor.  */
-      if (TREE_READONLY (exp)
+      if (0 && TREE_READONLY (exp)
          && !TREE_SIDE_EFFECTS (exp)
          && (modifier == EXPAND_NORMAL || modifier == EXPAND_STACK_PARM)
          && immediate_const_ctor_p (DECL_INITIAL (exp))


p.s.  I've just set up a build environment on another machine where I'm
using 
GNAT 11.3.1 20220421 (Red Hat 11.3.1-2)
as the bootstrap compiler, but it'll be a few hours before I'm investigating
in
gdb.

Many thanks again for your help.  Sorry again for the
breakage/inconvenience.

Roger
==



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PING] PR middle-end/95126: Expand small const structs as immediate constants
  2022-06-06 12:29         ` Roger Sayle
@ 2022-06-06 15:52           ` Rainer Orth
  0 siblings, 0 replies; 11+ messages in thread
From: Rainer Orth @ 2022-06-06 15:52 UTC (permalink / raw)
  To: Roger Sayle; +Cc: 'Andreas Schwab', gcc-patches

Hi Roger,

>> I just tried this on i386-pc-solaris2.11: unfortunately, it made no
> difference.
>
> Awesome!  Very many thanks for trying this.  Alas it confirms that the patch
> I'm currently spinning won't have any affect.  So by a process of
> elimination,
> the miscompilation must be triggered by one of the other two changes:
>
> diff --git a/gcc/expr.cc b/gcc/expr.cc
> index fb062dc..37f1405 100644
> --- a/gcc/expr.cc
> +++ b/gcc/expr.cc
> @@ -4871,7 +4871,7 @@ emit_push_insn (rtx x, machine_mode mode, tree type,
> rtx s
>           /* If source is a constant VAR_DECL with a simple constructor,
>               store the constructor to the stack instead of moving it.  */
>           const_tree decl;
> -         if (partial == 0
> +         if (0 && partial == 0
>               && MEM_P (xinner)
>               && SYMBOL_REF_P (XEXP (xinner, 0))
>               && (decl = SYMBOL_REF_DECL (XEXP (xinner, 0))) != NULL_TREE
> @@ -10603,7 +10603,7 @@ expand_expr_real_1 (tree exp, rtx target,
> machine_mode t
>         }
>        /* Expand const VAR_DECLs with CONSTRUCTOR initializers that
>          have scalar integer modes to a reg via store_constructor.  */
> -      if (TREE_READONLY (exp)
> +      if (0 && TREE_READONLY (exp)
>           && !TREE_SIDE_EFFECTS (exp)
>           && (modifier == EXPAND_NORMAL || modifier == EXPAND_STACK_PARM)
>           && immediate_const_ctor_p (DECL_INITIAL (exp))

I've tried both chunks individually: the second one allowed the build to
finish successfully.

Thanks.
        Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-06-06 15:52 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-21 10:49 [PING] PR middle-end/95126: Expand small const structs as immediate constants Roger Sayle
2022-06-05 17:53 ` Andreas Schwab
2022-06-05 20:31   ` Rainer Orth
2022-06-06  8:43     ` Roger Sayle
2022-06-06 11:16       ` Andreas Schwab
2022-06-06 11:23         ` Roger Sayle
2022-06-06 11:44           ` Andreas Schwab
2022-06-06 12:01         ` Rainer Orth
2022-06-06 12:00       ` Rainer Orth
2022-06-06 12:29         ` Roger Sayle
2022-06-06 15:52           ` Rainer Orth

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).