public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] go: Fix up go.test/test/fixedbugs/bug207.go failure [PR109258]
@ 2023-03-24  9:22 Jakub Jelinek
  2023-03-24 17:12 ` Ian Lance Taylor
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2023-03-24  9:22 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: gcc-patches

Hi!

The PR109086 r13-6690 inline_string_cmp change to
      if (diff != result)
        emit_move_insn (result, diff);
regressed
FAIL: go.test/test/fixedbugs/bug207.go,  -O2 -g  (internal compiler error: in emit_move_insn, at expr.cc:4224)
The problem is the Go FE doesn't mark __builtin_memcmp as pure as other FEs,
so we ended up with
  __builtin_memcmp (whatever, whateverelse, somesize);
in the IL before expansion and the expansion ICEd on it.
As the builtin calls a library function which is pure or is inline expanded
as such, not marking it pure is an unnecessary pessimization from the FE
side, keeping such dead calls in the IL if they aren't needed will not help
anything.

The following patch fixes that.  Initially I've added just DECL_PURE_P to
it, but that unfortunately broke bootstrap, for __builtin_memcmp there is
also __builtin_memcmp_eq registered by the middle-end code if not registered
earlier and that one is registered with the usual flags (pure, nothrow,
leaf), so if __builtin_memcmp from FE was just pure, it would appear in the
IL as that it can raise exceptions and when folded into __builtin_memcmp_eq
all of sudden it couldn't and we'd ICE in verification.

I think tons of functions should have builtin_nothrow as well, but changing
that wasn't necessary for this fix.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2023-03-24  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/109258
	* go-gcc.cc (Gcc_backend): Add new static data members builtin_pure
	and builtin_nothrow.
	(Gcc_backend::Gcc_backend): Pass builtin_pure | builtin_nothrow for
	BUILT_IN_MEMCMP.
	(Gcc_backend::define_builtin): Handle builtin_pure and builtin_nothrow
	in flags.

--- gcc/go/go-gcc.cc.jj	2023-01-18 12:22:10.396234744 +0100
+++ gcc/go/go-gcc.cc	2023-03-23 23:48:11.120064331 +0100
@@ -543,6 +543,8 @@ private:
   static const int builtin_const = 1 << 0;
   static const int builtin_noreturn = 1 << 1;
   static const int builtin_novops = 1 << 2;
+  static const int builtin_pure = 1 << 3;
+  static const int builtin_nothrow = 1 << 4;
 
   void
   define_builtin(built_in_function bcode, const char* name, const char* libname,
@@ -601,7 +603,7 @@ Gcc_backend::Gcc_backend()
 						const_ptr_type_node,
 						size_type_node,
 						NULL_TREE),
-		       0);
+		       builtin_pure | builtin_nothrow);
 
   // We use __builtin_memmove for copying data.
   this->define_builtin(BUILT_IN_MEMMOVE, "__builtin_memmove", "memmove",
@@ -3596,6 +3598,10 @@ Gcc_backend::define_builtin(built_in_fun
 				   libname, NULL_TREE);
   if ((flags & builtin_const) != 0)
     TREE_READONLY(decl) = 1;
+  if ((flags & builtin_pure) != 0)
+    DECL_PURE_P(decl) = 1;
+  if ((flags & builtin_nothrow) != 0)
+    TREE_NOTHROW (decl) = 1;
   if ((flags & builtin_noreturn) != 0)
     TREE_THIS_VOLATILE(decl) = 1;
   if ((flags & builtin_novops) != 0)
@@ -3608,6 +3614,10 @@ Gcc_backend::define_builtin(built_in_fun
 				  NULL, NULL_TREE);
       if ((flags & builtin_const) != 0)
 	TREE_READONLY(decl) = 1;
+      if ((flags & builtin_pure) != 0)
+	DECL_PURE_P(decl) = 1;
+      if ((flags & builtin_nothrow) != 0)
+	TREE_NOTHROW (decl) = 1;
       if ((flags & builtin_noreturn) != 0)
 	TREE_THIS_VOLATILE(decl) = 1;
       if ((flags & builtin_novops) != 0)

	Jakub


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

* Re: [PATCH] go: Fix up go.test/test/fixedbugs/bug207.go failure [PR109258]
  2023-03-24  9:22 [PATCH] go: Fix up go.test/test/fixedbugs/bug207.go failure [PR109258] Jakub Jelinek
@ 2023-03-24 17:12 ` Ian Lance Taylor
  0 siblings, 0 replies; 2+ messages in thread
From: Ian Lance Taylor @ 2023-03-24 17:12 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

Jakub Jelinek <jakub@redhat.com> writes:

> 2023-03-24  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR middle-end/109258
> 	* go-gcc.cc (Gcc_backend): Add new static data members builtin_pure
> 	and builtin_nothrow.
> 	(Gcc_backend::Gcc_backend): Pass builtin_pure | builtin_nothrow for
> 	BUILT_IN_MEMCMP.
> 	(Gcc_backend::define_builtin): Handle builtin_pure and builtin_nothrow
> 	in flags.

This is OK.  Thanks.  Go ahead and commit.

Ian

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

end of thread, other threads:[~2023-03-24 17:12 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-24  9:22 [PATCH] go: Fix up go.test/test/fixedbugs/bug207.go failure [PR109258] Jakub Jelinek
2023-03-24 17:12 ` Ian Lance Taylor

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