public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] ipa: Force args obtined through pass-through maps to the expected type (PR 113964)
@ 2024-04-05 12:53 Martin Jambor
  2024-04-05 12:54 ` Jan Hubicka
  0 siblings, 1 reply; 2+ messages in thread
From: Martin Jambor @ 2024-04-05 12:53 UTC (permalink / raw)
  To: GCC Patches; +Cc: Jan Hubicka

Hi,

interactions of IPA-CP and IPA-SRA on the same data is a rather big
source of issues, I'm afraid.  PR 113964 is a situation where IPA-CP
propagates an unsigned short in a union parameter into a function
which itself calls a different function which has a same union
parameter and both these union parameters are split with IPA-SRA.  The
leaf function however uses a signed short member of the union.

In the calling function, we get the unsigned constant as the
replacement for the union and it is then passed in the call without
any type compatibility checks.  Apparently on riscv64 it matters
whether the parameter is signed or unsigned short and so the leaf
function can see different values.

Fixed by using useless_type_conversion_p at the appropriate place and
if it fails, use force_value_to type as elsewhere in similar
situations.

Bootstrapped and tested on x86_64-linux, the reporter has also run the
testsuite with this patch on riscv64 and reported in Bugzilla there were
no issues.

OK for master and GCC 13?

Thanks,

Martin


gcc/ChangeLog:

2024-04-04  Martin Jambor  <mjambor@suse.cz>

	PR ipa/113964
	* ipa-param-manipulation.cc (ipa_param_adjustments::modify_call):
	Force values obtined through pass-through maps to the expected
	split type.

gcc/testsuite/ChangeLog:

2024-04-04  Patrick O'Neill  <patrick@rivosinc.com>
	    Martin Jambor  <mjambor@suse.cz>

	PR ipa/113964
	* gcc.dg/ipa/pr114247.c: New test.
---
 gcc/ipa-param-manipulation.cc       |  6 ++++++
 gcc/testsuite/gcc.dg/ipa/pr114247.c | 31 +++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/pr114247.c

diff --git a/gcc/ipa-param-manipulation.cc b/gcc/ipa-param-manipulation.cc
index 3e0df6a6f77..b4ca78b652e 100644
--- a/gcc/ipa-param-manipulation.cc
+++ b/gcc/ipa-param-manipulation.cc
@@ -740,6 +740,12 @@ ipa_param_adjustments::modify_call (cgraph_edge *cs,
 	  }
       if (repl)
 	{
+	  if (!useless_type_conversion_p(apm->type, repl->typed.type))
+	    {
+	      repl = force_value_to_type (apm->type, repl);
+	      repl = force_gimple_operand_gsi (&gsi, repl,
+					       true, NULL, true, GSI_SAME_STMT);
+	    }
 	  vargs.quick_push (repl);
 	  continue;
 	}
diff --git a/gcc/testsuite/gcc.dg/ipa/pr114247.c b/gcc/testsuite/gcc.dg/ipa/pr114247.c
new file mode 100644
index 00000000000..60aa2bc0122
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr114247.c
@@ -0,0 +1,31 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fsigned-char -fno-strict-aliasing -fwrapv" } */
+
+union a {
+  unsigned short b;
+  int c;
+  signed short d;
+};
+int e, f = 1, g;
+long h;
+const int **i;
+void j(union a k, int l, unsigned m) {
+  const int *a[100];
+  i = &a[0];
+  h = k.d;
+}
+static int o(union a k) {
+  k.d = -1;
+  while (1)
+    if (f)
+      break;
+  j(k, g, e);
+  return 0;
+}
+int main() {
+  union a n = {1};
+  o(n);
+  if (h != -1)
+    __builtin_abort();
+  return 0;
+}
-- 
2.44.0


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

* Re: [PATCH] ipa: Force args obtined through pass-through maps to the expected type (PR 113964)
  2024-04-05 12:53 [PATCH] ipa: Force args obtined through pass-through maps to the expected type (PR 113964) Martin Jambor
@ 2024-04-05 12:54 ` Jan Hubicka
  0 siblings, 0 replies; 2+ messages in thread
From: Jan Hubicka @ 2024-04-05 12:54 UTC (permalink / raw)
  To: Martin Jambor; +Cc: GCC Patches

> Hi,
> 
> interactions of IPA-CP and IPA-SRA on the same data is a rather big
> source of issues, I'm afraid.  PR 113964 is a situation where IPA-CP
> propagates an unsigned short in a union parameter into a function
> which itself calls a different function which has a same union
> parameter and both these union parameters are split with IPA-SRA.  The
> leaf function however uses a signed short member of the union.
> 
> In the calling function, we get the unsigned constant as the
> replacement for the union and it is then passed in the call without
> any type compatibility checks.  Apparently on riscv64 it matters
> whether the parameter is signed or unsigned short and so the leaf
> function can see different values.
> 
> Fixed by using useless_type_conversion_p at the appropriate place and
> if it fails, use force_value_to type as elsewhere in similar
> situations.
> 
> Bootstrapped and tested on x86_64-linux, the reporter has also run the
> testsuite with this patch on riscv64 and reported in Bugzilla there were
> no issues.
> 
> OK for master and GCC 13?
> 
> Thanks,
> 
> Martin
> 
> 
> gcc/ChangeLog:
> 
> 2024-04-04  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR ipa/113964
> 	* ipa-param-manipulation.cc (ipa_param_adjustments::modify_call):
> 	Force values obtined through pass-through maps to the expected
> 	split type.
> 
> gcc/testsuite/ChangeLog:
> 
> 2024-04-04  Patrick O'Neill  <patrick@rivosinc.com>
> 	    Martin Jambor  <mjambor@suse.cz>
> 
> 	PR ipa/113964
> 	* gcc.dg/ipa/pr114247.c: New test.
OK,
thanks!
Honza

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

end of thread, other threads:[~2024-04-05 12:54 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-05 12:53 [PATCH] ipa: Force args obtined through pass-through maps to the expected type (PR 113964) Martin Jambor
2024-04-05 12:54 ` Jan Hubicka

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