[patch] Fix PR middle-end/55321

[patch] Fix PR middle-end/55321

[Eric Botcazou]

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

Hi,

this is the build failure of the Ada runtime on the ARM:

+===========================GNAT BUG DETECTED==============================+
| 4.8.0 20121111 (experimental) (armv5tel-unknown-linux-gnueabi) GCC error:|
| in merge_latch_edges, at cfgloop.c:678                                   |
| Error detected around s-stposu.adb:565:8                                 |
| Please submit a bug report; see http://gcc.gnu.org/bugs.html.     

The problem is that get_loop_latch_edges finds no latch edges for a loop 
because the header of the loop doesn't dominate any of its predecessors.
The reason is that a new edge is added during RTL expansion, which changes the 
dominance info.  This edge is from a call to __sync_synchronize to a non-local 
label generated for a __builtin_setjmp_receiver.

Fixed by marking the call as "nononlocal", tested on x86_64-suse-linux, OK for 
the mainline?


2012-11-15  Eric Botcazou  <ebotcazou@adacore.com>

	PR middle-end/55321
	* optabs.c (mark_nothrow_nononlocal): New function extracted from...
	(emit_libcall_block_1): ...here.  Call it.
	(expand_mem_thread_fence): Call it for synchronize_libfunc.


2012-11-15  Eric Botcazou  <ebotcazou@adacore.com>

	* loop_optimization14.ad[sb]: New test.
	* loop_optimization14_pkg.ads: New helper.


-- 
Eric Botcazou

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

Index: optabs.c
===================================================================
--- optabs.c	(revision 193528)
+++ optabs.c	(working copy)
@@ -3838,6 +3838,23 @@ no_conflict_move_test (rtx dest, const_r
 }
 
 \f
+/* Look for any CALL_INSNs in the insn chain starting at INSN and attach a
+   REG_EH_REGION reg note to indicate that this call cannot throw or execute
+   a nonlocal goto (unless there is already a REG_EH_REGION note, in which
+   case we update it).  */
+
+static void
+mark_nothrow_nononlocal (rtx insn)
+{
+  while (insn)
+    {
+      if (CALL_P (insn))
+	make_reg_eh_region_note_nothrow_nononlocal (insn);
+
+      insn = NEXT_INSN (insn);
+    }
+}
+
 /* Emit code to make a call to a constant function or a library call.
 
    INSNS is a list containing all insns emitted in the call.
@@ -3881,15 +3898,7 @@ emit_libcall_block_1 (rtx insns, rtx tar
 	  }
     }
   else
-    {
-      /* Look for any CALL_INSNs in this sequence, and attach a REG_EH_REGION
-	 reg note to indicate that this call cannot throw or execute a nonlocal
-	 goto (unless there is already a REG_EH_REGION note, in which case
-	 we update it).  */
-      for (insn = insns; insn; insn = NEXT_INSN (insn))
-	if (CALL_P (insn))
-	  make_reg_eh_region_note_nothrow_nononlocal (insn);
-    }
+    mark_nothrow_nononlocal (insns);
 
   /* First emit all insns that set pseudos.  Remove them from the list as
      we go.  Avoid insns that set pseudos which were referenced in previous
@@ -7404,7 +7413,15 @@ expand_mem_thread_fence (enum memmodel m
       if (HAVE_memory_barrier)
 	emit_insn (gen_memory_barrier ());
       else if (synchronize_libfunc != NULL_RTX)
-	emit_library_call (synchronize_libfunc, LCT_NORMAL, VOIDmode, 0);
+	{
+	  rtx insns;
+	  start_sequence ();
+	  emit_library_call (synchronize_libfunc, LCT_NORMAL, VOIDmode, 0);
+	  insns = get_insns ();
+	  end_sequence ();
+	  mark_nothrow_nononlocal (insns);
+	  emit_insn (insns);
+	}
       else
 	expand_asm_memory_barrier ();
     }

[-- Attachment #3: loop_optimization14.adb --]
[-- Type: text/x-adasrc, Size: 554 bytes --]

-- PR middle-end/55321
-- { dg-do compile }
-- { dg-options "-O" }

with Loop_Optimization14_Pkg; use Loop_Optimization14_Pkg;

package body Loop_Optimization14 is

   procedure Finalize_Pool (Pool : in out Rec) is
      Raised : Boolean := False;
   begin
      Pool.A := True;

      while not Pool.B loop

         begin
            Proc (Pool.B);

         exception
            when others =>
               if not Raised then
                  Raised := True;
               end if;
         end;
      end loop;

   end;

end Loop_Optimization14;

[-- Attachment #4: loop_optimization14.ads --]
[-- Type: text/x-adasrc, Size: 209 bytes --]

package Loop_Optimization14 is

   type Rec is record
      A : Boolean;
      pragma Atomic (A);

      B : Boolean;

   end record;

   procedure Finalize_Pool (Pool : in out Rec);

end Loop_Optimization14;

[-- Attachment #5: loop_optimization14_pkg.ads --]
[-- Type: text/x-adasrc, Size: 106 bytes --]

package Loop_Optimization14_Pkg is

   procedure Proc (B : in out Boolean);

end Loop_Optimization14_Pkg;

Re: [patch] Fix PR middle-end/55321

[Richard Henderson]

On 11/15/2012 11:34 AM, Eric Botcazou wrote:
> The problem is that get_loop_latch_edges finds no latch edges for a loop 
> because the header of the loop doesn't dominate any of its predecessors.
> The reason is that a new edge is added during RTL expansion, which changes the 
> dominance info.  This edge is from a call to __sync_synchronize to a non-local 
> label generated for a __builtin_setjmp_receiver.
> 
> Fixed by marking the call as "nononlocal", tested on x86_64-suse-linux, OK for 
> the mainline?

While its true that this call is nononlocal, there are plenty of other
builtins that we expand with just emit_library_call (as opposed to
emit_libcall_block) that also need this sort of treatment.

Seems to me that this marking ought to be done in emit_library_call,
for at least most values of LCT_*.  Otherwise you could get the same
ICE for e.g. memcpy.


r~


PS: ARM still uses sjlj exceptions for Ada?  I thought with the obsolescence
of pre-eabi targets that we'd always use unwind tables now.

Re: [patch] Fix PR middle-end/55321

[Ramana Radhakrishnan]

> r~
>
>
> PS: ARM still uses sjlj exceptions for Ada?  I thought with the obsolescence
> of pre-eabi targets that we'd always use unwind tables now.
>

I believe this is by choice because no one has yet written an unwinder 
for the ARM exception tables for Ada :( .

regards,
Ramana

Re: [patch] Fix PR middle-end/55321

[Richard Henderson]

On 11/15/2012 12:01 PM, Ramana Radhakrishnan wrote:
> I believe this is by choice because no one has yet written an unwinder for the ARM exception tables for Ada :( .

Ada is supposed to be using the same libgcc unwinder as the rest of
the compiler.  So this cannot be it, exactly.

Perhaps some silly bit of configury is wrong?


r~

Re: [patch] Fix PR middle-end/55321

[Ramana Radhakrishnan]

On 11/15/12 20:05, Richard Henderson wrote:
> On 11/15/2012 12:01 PM, Ramana Radhakrishnan wrote:
>> I believe this is by choice because no one has yet written an unwinder for the ARM exception tables for Ada :( .
>
> Ada is supposed to be using the same libgcc unwinder as the rest of
> the compiler.  So this cannot be it, exactly.

I don't follow Ada much, so if it's not the unwinder then the next place 
to look would be the generator.

Googling around -
http://gcc.gnu.org/ml/gcc-patches/2009-09/msg00471.html appears to 
suggest that GNAT needs to know more to generate the ARM EH unwind 
tables for the ZCX approach.

On Aarch64 life should be much simpler given that it uses the standard 
eh_frame mechanisms.


>
> Perhaps some silly bit of configury is wrong?

If things have changed maybe it is ....

Ramana

>
>
> r~
>

Re: [patch] Fix PR middle-end/55321

[Eric Botcazou]

> While its true that this call is nononlocal, there are plenty of other
> builtins that we expand with just emit_library_call (as opposed to
> emit_libcall_block) that also need this sort of treatment.

I'm a little skeptical, although __sync_synchronize is probably not the only 
one indeed.  I'll experiment.

> Seems to me that this marking ought to be done in emit_library_call,
> for at least most values of LCT_*.  Otherwise you could get the same
> ICE for e.g. memcpy.

For a naked call to memcpy?  I don't think so, this is a call so there will be 
an abnormal edge at the Tree level.  The problem arises only when the abnormal 
edge is created during RTL expansion.

> PS: ARM still uses sjlj exceptions for Ada?  I thought with the obsolescence
> of pre-eabi targets that we'd always use unwind tables now.

Yes, that's the last one.  Nobody has adapted (or submitted to the FSF) the 
#ifdef __ARM_EABI_UNWINDER__ / #endif adjustments present in libsupc++ or 
libjava.

-- 
Eric Botcazou

Re: [patch] Fix PR middle-end/55321

[Richard Henderson]

On 11/15/2012 03:08 PM, Eric Botcazou wrote:
> For a naked call to memcpy?  I don't think so, this is a call so there will be 
> an abnormal edge at the Tree level.  The problem arises only when the abnormal 
> edge is created during RTL expansion.

Well, there's a call at the tree level for __sync_synchronize as well.

So the question is: why does __s_s ICE but memcmp doesn't? I suppose
it's the mere rarity with which HAVE_cmpmemsi is defined, and expansion
of gen_cmpmemsi fails.

There are plenty of other calls to emit_library_call_value that are 
not subsequently protected by emit_libcall_block_1, but they are probably
sufficiently rare on common platforms that they'd be difficult to trigger.


r~

Re: [patch] Fix PR middle-end/55321

[Eric Botcazou]

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

> Well, there's a call at the tree level for __sync_synchronize as well.
> 
> So the question is: why does __s_s ICE but memcmp doesn't? I suppose
> it's the mere rarity with which HAVE_cmpmemsi is defined, and expansion
> of gen_cmpmemsi fails.
> 
> There are plenty of other calls to emit_library_call_value that are
> not subsequently protected by emit_libcall_block_1, but they are probably
> sufficiently rare on common platforms that they'd be difficult to trigger.

You're right, here's a patch that marks all no-throw libcalls as no-nonlocal.

Tested on x86_64-suse-linux, OK for the mainline?


2012-11-29  Eric Botcazou  <ebotcazou@adacore.com>

	PR middle-end/55321
	* calls.c (emit_library_call_value_1): Mark as no-nonlocal if no-throw.


-- 
Eric Botcazou

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

Index: calls.c
===================================================================
--- calls.c	(revision 193921)
+++ calls.c	(working copy)
@@ -4196,13 +4196,11 @@ emit_library_call_value_1 (int retval, r
      that it should complain if nonvolatile values are live.  For
      functions that cannot return, inform flow that control does not
      fall through.  */
-
   if (flags & ECF_NORETURN)
     {
       /* The barrier note must be emitted
 	 immediately after the CALL_INSN.  Some ports emit more than
 	 just a CALL_INSN above, so we must search for it here.  */
-
       rtx last = get_last_insn ();
       while (!CALL_P (last))
 	{
@@ -4214,6 +4212,21 @@ emit_library_call_value_1 (int retval, r
       emit_barrier_after (last);
     }
 
+  /* Consider that "regular" libcalls, i.e. all of them except for LCT_THROW
+     and LCT_RETURNS_TWICE, cannot perform non-local gotos.  */
+  if (flags & ECF_NOTHROW)
+    {
+      rtx last = get_last_insn ();
+      while (!CALL_P (last))
+	{
+	  last = PREV_INSN (last);
+	  /* There was no CALL_INSN?  */
+	  gcc_assert (last != before_call);
+	}
+
+      make_reg_eh_region_note_nothrow_nononlocal (last);
+    }
+
   /* Now restore inhibit_defer_pop to its actual original value.  */
   OK_DEFER_POP;
 

Re: [patch] Fix PR middle-end/55321

[Richard Henderson]

On 2012-11-29 06:29, Eric Botcazou wrote:
> 	PR middle-end/55321
> 	* calls.c (emit_library_call_value_1): Mark as no-nonlocal if no-throw.

Ok.


r~