V3 [PATCH] i386: Handle REG_EH_REGION note

Message ID CAMe9rOqimaZ+isqasQCG4-PxH1RbaBnz8Yn-wsgj4dKDpi3NYw@mail.gmail.com
State New
Headers show
Series
  • V3 [PATCH] i386: Handle REG_EH_REGION note
Related show

Commit Message

H.J. Lu March 14, 2019, 1:08 p.m.
On 3/14/19, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Mar 14, 2019 at 4:34 PM Jakub Jelinek <jakub@redhat.com> wrote:

>>

>> On Thu, Mar 14, 2019 at 11:30:21AM +0800, H.J. Lu wrote:

>> > We need to split the basic block if we create new insns, which may

>> > throw exceptions, in the middle of the basic blocks.

>> >

>> > Tested on AVX2 and AVX512 machines with and without

>> >

>> > --with-arch=native

>> >

>> > OK for trunk?

>>

>> That looks much better, I see you chose to follow what lower-subreg.c

>> does

>> here.  I just wonder if instead of the sbitmap of blocks to check for

>> splitting it

>> wouldn't be more efficient to have an auto_vec<rtx_insn *> holding the

>> exact

>> set_insns that need splitting after them and just grab the bb using

>> BLOCK_FOR_INSN.

>>

>

> Good idea.  I will work on it.

>


Here it is.  OK for trunk?

Thanks.

-- 
H.J.

Comments

Jakub Jelinek March 14, 2019, 1:22 p.m. | #1
On Thu, Mar 14, 2019 at 09:08:17PM +0800, H.J. Lu wrote:
> --- a/gcc/config/i386/i386.c

> +++ b/gcc/config/i386/i386.c

> @@ -2819,6 +2819,8 @@ remove_partial_avx_dependency (void)

>    rtx set;

>    rtx v4sf_const0 = NULL_RTX;

>  

> +  auto_vec<rtx_insn *> splitted_insn;


Perhaps throwing_insns or flow_transfer_insns or control_flow_insns
instead?

> +	  unsigned int i;

> +	  FOR_EACH_VEC_ELT (splitted_insn, i, insn)


I actually meant something like:
	    if (control_flow_insn_p (insn))
	      {
		/* Split the block after insn.  There will be a
		   fallthru edge, which is OK so we keep it.  We
		   have to create the exception edges ourselves.  */
		split_block (bb, insn);
		rtl_make_eh_edge (NULL, bb, BB_END (bb));
	      }

only inside of the FOR_EACH_VEC_ELT.  If there is more than one
insn that needs splitting in the bb, it will be in the vector as well
and handled in some later iteration.

	Jakub

Patch

From 64ac61ce7095c49406ea34b05f1c22fbb98ca3f7 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 12 Mar 2019 08:55:24 +0800
Subject: [PATCH] i386: Handle REG_EH_REGION note

When we split:

(insn 18 17 76 2 (set (reg:SF 88 [ _19 ])
        (float:SF (mem/c:SI (symbol_ref:DI ("d") [flags 0x2]  <var_decl 0x7fd6d8290c60 d>) [1 d+0 S4 A32]))) "x.ii":4:20 170 {*floatsisf2}
     (expr_list:REG_EH_REGION (const_int 2 [0x2])
        (nil)))

to

(insn 94 17 18 2 (set (reg:V4SF 115)
        (vec_merge:V4SF (vec_duplicate:V4SF (float:SF (mem/c:SI (symbol_ref:DI ("d") [flags 0x2]  <var_decl 0x7f346837ac60 d>) [1 d+0 S4 A32])))
            (reg:V4SF 114)
            (const_int 1 [0x1]))) "x.ii":4:20 -1
     (nil))
(insn 18 94 76 2 (set (reg:SF 88 [ _19 ])
        (subreg:SF (reg:V4SF 115) 0)) "x.ii":4:20 112 {*movsf_internal}
     (expr_list:REG_EH_REGION (const_int 2 [0x2])
        (nil)))

we must copy the REG_EH_REGION note to the first insn and split the block
after the newly added insn.  The REG_EH_REGION on the second insn will be
removed later since it no longer traps.

gcc/

	PR target/89650
	* config/i386/i386.c (remove_partial_avx_dependency): Handle
	REG_EH_REGION note.

gcc/testsuite/

	PR target/89650
	* g++.target/i386/pr89650.C: New test.
---
 gcc/config/i386/i386.c                  | 42 +++++++++++++++++++++++++
 gcc/testsuite/g++.target/i386/pr89650.C | 19 +++++++++++
 2 files changed, 61 insertions(+)
 create mode 100644 gcc/testsuite/g++.target/i386/pr89650.C

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 1f94a45909d..93ff46f329a 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2819,6 +2819,8 @@  remove_partial_avx_dependency (void)
   rtx set;
   rtx v4sf_const0 = NULL_RTX;
 
+  auto_vec<rtx_insn *> splitted_insn;
+
   FOR_EACH_BB_FN (bb, cfun)
     {
       FOR_BB_INSNS (bb, insn)
@@ -2875,6 +2877,17 @@  remove_partial_avx_dependency (void)
 	  set_insn = emit_insn_before (set, insn);
 	  df_insn_rescan (set_insn);
 
+	  if (cfun->can_throw_non_call_exceptions)
+	    {
+	      /* Handle REG_EH_REGION note.  */
+	      rtx note = find_reg_note (insn, REG_EH_REGION, NULL_RTX);
+	      if (note)
+		{
+		  splitted_insn.safe_push (set_insn);
+		  add_reg_note (set_insn, REG_EH_REGION, XEXP (note, 0));
+		}
+	    }
+
 	  src = gen_rtx_SUBREG (dest_mode, vec, 0);
 	  set = gen_rtx_SET (dest, src);
 
@@ -2925,6 +2938,35 @@  remove_partial_avx_dependency (void)
       df_insn_rescan (set_insn);
       df_process_deferred_rescans ();
       loop_optimizer_finalize ();
+
+      if (!splitted_insn.is_empty ())
+	{
+	  free_dominance_info (CDI_DOMINATORS);
+
+	  unsigned int i;
+	  FOR_EACH_VEC_ELT (splitted_insn, i, insn)
+	    {
+	      rtx_insn *end;
+	      edge fallthru;
+
+	      bb = BLOCK_FOR_INSN (insn);
+	      end = BB_END (bb);
+
+	      while (insn != end)
+		if (control_flow_insn_p (insn))
+		  {
+		    /* Split the block after insn.  There will be a
+		       fallthru edge, which is OK so we keep it.  We
+		       have to create the exception edges ourselves.  */
+		    fallthru = split_block (bb, insn);
+		    rtl_make_eh_edge (NULL, bb, BB_END (bb));
+		    bb = fallthru->dest;
+		    insn = BB_HEAD (bb);
+		  }
+		else
+		  insn = NEXT_INSN (insn);
+	    }
+	}
     }
 
   bitmap_obstack_release (NULL);
diff --git a/gcc/testsuite/g++.target/i386/pr89650.C b/gcc/testsuite/g++.target/i386/pr89650.C
new file mode 100644
index 00000000000..4b253cbf467
--- /dev/null
+++ b/gcc/testsuite/g++.target/i386/pr89650.C
@@ -0,0 +1,19 @@ 
+// { dg-do compile { target c++11 } }
+// { dg-options "-O2 -flive-range-shrinkage -fno-tree-dce -fno-dce -fnon-call-exceptions -mavx" }
+
+int d, e;
+struct g {
+  float f;
+  g(float h) : f(h + d) {}
+  ~g() {}
+};
+struct i {
+  int a;
+  int b : 4;
+  int &c;
+  i(int h) : a(), b(), c(h) {}
+};
+int main() {
+  i j(e);
+  g k[]{1, 2};
+}
-- 
2.20.1