public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: [PATCH] middle-end/100786 - constant folding from incompatible alias
@ 2022-03-26 16:54 FX
  2022-03-28  6:34 ` Richard Biener
  0 siblings, 1 reply; 6+ messages in thread
From: FX @ 2022-03-26 16:54 UTC (permalink / raw)
  To: rguenther; +Cc: gcc-patches, Iain Sandoe

Hi Richard,

The patch for PR100786 introduced a testcase that systematically fails on darwin:

FAIL: gcc.dg/torture/pr100786.c   -O0  (test for excess errors)
FAIL: gcc.dg/torture/pr100786.c   -O1  (test for excess errors)
FAIL: gcc.dg/torture/pr100786.c   -O2  (test for excess errors)
FAIL: gcc.dg/torture/pr100786.c   -O2 -flto  (test for excess errors)
FAIL: gcc.dg/torture/pr100786.c   -O2 -flto -flto-partition=none  (test for excess errors)
FAIL: gcc.dg/torture/pr100786.c   -O3 -g  (test for excess errors)
FAIL: gcc.dg/torture/pr100786.c   -Os  (test for excess errors)

see https://gcc.gnu.org/pipermail/gcc-testresults/2022-March/757882.html

because

pr100786.c:4:12: error: only weak aliases are supported in this configuration
    4 | extern int b __attribute__((alias("a")));
      |            ^
pr100786.c:8:15: error: only weak aliases are supported in this configuration
    8 | extern double b2 __attribute__((alias("a2")));
      |               ^~


FX

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

* Re: [PATCH] middle-end/100786 - constant folding from incompatible alias
  2022-03-26 16:54 [PATCH] middle-end/100786 - constant folding from incompatible alias FX
@ 2022-03-28  6:34 ` Richard Biener
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2022-03-28  6:34 UTC (permalink / raw)
  To: FX; +Cc: gcc-patches, Iain Sandoe

On Sat, 26 Mar 2022, FX wrote:

> Hi Richard,
> 
> The patch for PR100786 introduced a testcase that systematically fails on darwin:
> 
> FAIL: gcc.dg/torture/pr100786.c   -O0  (test for excess errors)
> FAIL: gcc.dg/torture/pr100786.c   -O1  (test for excess errors)
> FAIL: gcc.dg/torture/pr100786.c   -O2  (test for excess errors)
> FAIL: gcc.dg/torture/pr100786.c   -O2 -flto  (test for excess errors)
> FAIL: gcc.dg/torture/pr100786.c   -O2 -flto -flto-partition=none  (test for excess errors)
> FAIL: gcc.dg/torture/pr100786.c   -O3 -g  (test for excess errors)
> FAIL: gcc.dg/torture/pr100786.c   -Os  (test for excess errors)
> 
> see https://gcc.gnu.org/pipermail/gcc-testresults/2022-March/757882.html
> 
> because
> 
> pr100786.c:4:12: error: only weak aliases are supported in this configuration
>     4 | extern int b __attribute__((alias("a")));
>       |            ^
> pr100786.c:8:15: error: only weak aliases are supported in this configuration
>     8 | extern double b2 __attribute__((alias("a2")));
>       |               ^~

I'll add

/* { dg-require-alias "" } */

Richard.

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

* Re: [PATCH] middle-end/100786 - constant folding from incompatible alias
  2022-01-20 14:21   ` Richard Biener
@ 2022-01-20 15:10     ` Richard Biener
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2022-01-20 15:10 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On Thu, 20 Jan 2022, Richard Biener wrote:

> On Thu, 20 Jan 2022, Jakub Jelinek wrote:
> 
> > On Thu, Jan 20, 2022 at 02:58:21PM +0100, Richard Biener via Gcc-patches wrote:
> > > The following avoids us ICEing doing constant folding from variables
> > > with aliases of different types.  The formerly used fold_convert
> > > wasn't entirely correct even for the cases it handled and using
> > > a VIEW_CONVERT_EXPR avoids the ICE.  Reading from a larger alias
> > > will cause unfolded constants to appear but appearantly we handle
> > > that just "fine".
> > > 
> > >   b.0_1 = VIEW_CONVERT_EXPR<double>(1);
> > 
> > If they have the same sizes, why not, but doesn't int have
> > different size from double and isn't VCE defined only for same sizes?
> 
> Well yes, it's undefined.  IL wise we only constrain us for SSA
> operands, not constants.  But the whole testcase is undefined,
> and previously we'd happily accept a int -1 as a -1 long by
> sign-extending it.
> 
> I'm going to test an alternative patch tackling get_symbol_constant_value
> which is only used from CCP (which looks suffering from the same issue)
> and folding.

Like the following.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Richard.

From cc3f3c7428253c2326a00699f08bd89467b941f5 Mon Sep 17 00:00:00 2001
From: Richard Biener <rguenther@suse.de>
Date: Thu, 20 Jan 2022 14:25:51 +0100
Subject: [PATCH] middle-end/100786 - constant folding from incompatible alias
To: gcc-patches@gcc.gnu.org

The following avoids us ICEing doing constant folding from variables
with aliases of different types.  The issue appears both in
folding and CCP and FRE can do more fancy stuff to still constant
fold cases where the load is smaller than the initializer so
defer it to there.

2022-01-20  Richard Biener  <rguenther@suse.de>

	PR middle-end/100786
	* gimple-fold.cc (get_symbol_constant_value): Only return
	values of compatible type to the symbol.

	* gcc.dg/torture/pr100786.c: New testcase.
---
 gcc/gimple-fold.cc                      | 4 +++-
 gcc/testsuite/gcc.dg/torture/pr100786.c | 9 +++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr100786.c

diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
index 08d3cc214ff..3639aa20626 100644
--- a/gcc/gimple-fold.cc
+++ b/gcc/gimple-fold.cc
@@ -291,7 +291,9 @@ get_symbol_constant_value (tree sym)
       if (val)
 	{
 	  val = canonicalize_constructor_val (unshare_expr (val), sym);
-	  if (val && is_gimple_min_invariant (val))
+	  if (val
+	      && is_gimple_min_invariant (val)
+	      && useless_type_conversion_p (TREE_TYPE (sym), TREE_TYPE (val)))
 	    return val;
 	  else
 	    return NULL_TREE;
diff --git a/gcc/testsuite/gcc.dg/torture/pr100786.c b/gcc/testsuite/gcc.dg/torture/pr100786.c
new file mode 100644
index 00000000000..42f4e485593
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr100786.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+
+const double a = 0;
+extern int b __attribute__((alias("a")));
+void inc() { b++; }
+
+const int a2 = 0;
+extern double b2 __attribute__((alias("a2")));
+void inc2() { b2+=1; }
-- 
2.31.1


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

* Re: [PATCH] middle-end/100786 - constant folding from incompatible alias
  2022-01-20 14:02 ` Jakub Jelinek
@ 2022-01-20 14:21   ` Richard Biener
  2022-01-20 15:10     ` Richard Biener
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2022-01-20 14:21 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On Thu, 20 Jan 2022, Jakub Jelinek wrote:

> On Thu, Jan 20, 2022 at 02:58:21PM +0100, Richard Biener via Gcc-patches wrote:
> > The following avoids us ICEing doing constant folding from variables
> > with aliases of different types.  The formerly used fold_convert
> > wasn't entirely correct even for the cases it handled and using
> > a VIEW_CONVERT_EXPR avoids the ICE.  Reading from a larger alias
> > will cause unfolded constants to appear but appearantly we handle
> > that just "fine".
> > 
> >   b.0_1 = VIEW_CONVERT_EXPR<double>(1);
> 
> If they have the same sizes, why not, but doesn't int have
> different size from double and isn't VCE defined only for same sizes?

Well yes, it's undefined.  IL wise we only constrain us for SSA
operands, not constants.  But the whole testcase is undefined,
and previously we'd happily accept a int -1 as a -1 long by
sign-extending it.

I'm going to test an alternative patch tackling get_symbol_constant_value
which is only used from CCP (which looks suffering from the same issue)
and folding.

Richard.

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

* Re: [PATCH] middle-end/100786 - constant folding from incompatible alias
  2022-01-20 13:58 Richard Biener
@ 2022-01-20 14:02 ` Jakub Jelinek
  2022-01-20 14:21   ` Richard Biener
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2022-01-20 14:02 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

On Thu, Jan 20, 2022 at 02:58:21PM +0100, Richard Biener via Gcc-patches wrote:
> The following avoids us ICEing doing constant folding from variables
> with aliases of different types.  The formerly used fold_convert
> wasn't entirely correct even for the cases it handled and using
> a VIEW_CONVERT_EXPR avoids the ICE.  Reading from a larger alias
> will cause unfolded constants to appear but appearantly we handle
> that just "fine".
> 
>   b.0_1 = VIEW_CONVERT_EXPR<double>(1);

If they have the same sizes, why not, but doesn't int have
different size from double and isn't VCE defined only for same sizes?

	Jakub


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

* [PATCH] middle-end/100786 - constant folding from incompatible alias
@ 2022-01-20 13:58 Richard Biener
  2022-01-20 14:02 ` Jakub Jelinek
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2022-01-20 13:58 UTC (permalink / raw)
  To: gcc-patches

The following avoids us ICEing doing constant folding from variables
with aliases of different types.  The formerly used fold_convert
wasn't entirely correct even for the cases it handled and using
a VIEW_CONVERT_EXPR avoids the ICE.  Reading from a larger alias
will cause unfolded constants to appear but appearantly we handle
that just "fine".

  b.0_1 = VIEW_CONVERT_EXPR<double>(1);

There is no obvious spot and way to just disable the constant folding
so I've not attempted to do that.

Boostrapped on x86_64-unknown-linux-gnu, testing in progress.

2022-01-20  Richard Biener  <rguenther@suse.de>

	PR middle-end/100786
	* gimple-fold.cc (fold_stmt_1): Use a VIEW_CONVERT_EXPR on type
	mismatches.

	* gcc.dg/torture/pr100786.c: New testcase.
---
 gcc/gimple-fold.cc                      | 2 +-
 gcc/testsuite/gcc.dg/torture/pr100786.c | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr100786.c

diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
index 08d3cc214ff..a4f1fdabc18 100644
--- a/gcc/gimple-fold.cc
+++ b/gcc/gimple-fold.cc
@@ -6264,7 +6264,7 @@ fold_stmt_1 (gimple_stmt_iterator *gsi, bool inplace, tree (*valueize) (tree))
 	if (new_rhs
 	    && !useless_type_conversion_p (TREE_TYPE (lhs),
 					   TREE_TYPE (new_rhs)))
-	  new_rhs = fold_convert (TREE_TYPE (lhs), new_rhs);
+	  new_rhs = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (lhs), new_rhs);
 	if (new_rhs
 	    && (!inplace
 		|| get_gimple_rhs_num_ops (TREE_CODE (new_rhs)) < old_num_ops))
diff --git a/gcc/testsuite/gcc.dg/torture/pr100786.c b/gcc/testsuite/gcc.dg/torture/pr100786.c
new file mode 100644
index 00000000000..42f4e485593
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr100786.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+
+const double a = 0;
+extern int b __attribute__((alias("a")));
+void inc() { b++; }
+
+const int a2 = 0;
+extern double b2 __attribute__((alias("a2")));
+void inc2() { b2+=1; }
-- 
2.31.1

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

end of thread, other threads:[~2022-03-28  6:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-26 16:54 [PATCH] middle-end/100786 - constant folding from incompatible alias FX
2022-03-28  6:34 ` Richard Biener
  -- strict thread matches above, loose matches on Subject: below --
2022-01-20 13:58 Richard Biener
2022-01-20 14:02 ` Jakub Jelinek
2022-01-20 14:21   ` Richard Biener
2022-01-20 15:10     ` Richard Biener

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