diff mbox

PR target/71549: Convert V1TImode register to TImode in debug insn

Message ID CAMe9rOq3=sMc5wPzmo2ker1yFDXF2BdmYmtZUgg5jF_H2Pfq9g@mail.gmail.com
State New
Headers show

Commit Message

H.J. Lu June 20, 2016, 4:45 p.m. UTC 
On Mon, Jun 20, 2016 at 7:30 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2016-06-20 16:39 GMT+03:00 Uros Bizjak <ubizjak@gmail.com>:
>> On Mon, Jun 20, 2016 at 1:55 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>>> TImode register referenced in debug insn can be converted to V1TImode
>>> by scalar to vector optimization.  We need to convert a debug insn if
>>> it has a variable in a TImode register.
>>>
>>> Tested on x86-64.  OK for trunk?
>>
>> Ilya, does this approach look good to you? Also, does DImode STV
>> conversion need similar handling of debug insns?
>
> DImode conversion doesn't change register mode (i.e. never calls
> PUT_MODE for registers).  That keeps debug instructions valid.
>
> Overall I don't like the idea of having debug insns in candidates
> set and in chains.  Looks like it is possible to have a chain
> consisting of a debug insn only which is weird (otherwise I don't
> see why we may return false in timode_scalar_chain::convert_insn).

Yes, it can happen:

(insn 11 8 12 2 (parallel [
            (set (reg/v:TI 91 [ <retval> ])
                (plus:TI (reg/v:TI 92 [ a ])
                    (reg/v:TI 96 [ b ])))
            (clobber (reg:CC 17 flags))
        ]) y.i:5 210 {*addti3_doubleword}
     (expr_list:REG_UNUSED (reg:CC 17 flags)
        (nil)))
(debug_insn 12 11 13 2 (var_location:TI w (reg/v:TI 91 [ <retval> ])) y.i:5 -1
     (nil))


> What about other possible register uses?  If debug insns are added
> to candidates then NONDEBUG_INSN_P check for uses in
> timode_check_non_convertible_regs becomes invalid, right?

Debug insn has no impact on STV decision.  We just need to convert
register referenced in debug insn from V1TImode to TImode in
timode_scalar_chain::convert_insn.

> If we have (or want) to fix some register uses then it's probably
> would be better to visit register uses when we convert its mode
> and make required fix-ups.  It seems better to me to not involve
> debug insns in analysis phase.

Here is the updated patch to add debug insn, which references the
TImode register which will be converted to V1TImode to queue.
I am testing it now.

> Also I don't think debug insns should be accounted as optimized
> instructions because we would get different number of optimized
> instructions depending on debug info availability which may be
> inconvenient for dump scans (and it is not a real instruction
> optimization).
>
> Thanks,
> Ilya
>
>>
>> Uros.
>>
>>>
>>> H.J.
>>> ----
>>> gcc/
>>>
>>>         PR target/71549
>>>         * config/i386/i386.c (timode_scalar_to_vector_candidate_p):
>>>         Return true if debug insn has a variable in TImode register.
>>>         (timode_remove_non_convertible_regs): Skip debug insn.
>>>         (scalar_chain::convert_insn): Change return type to bool.
>>>         (scalar_chain::add_insn): Don't check registers in debug insn.
>>>         (dimode_scalar_chain::convert_insn): Change return type to bool
>>>         and always return true.
>>>         (timode_scalar_chain::convert_insn): Change return type to bool.
>>>         Convert V1TImode register to SUBREG TImode in debug insn.  Return
>>>         false if debug insn isn't converted.  Otherwise, return true.
>>>         (scalar_chain::convert): Increment converted_insns only if
>>>         convert_insn returns true.
>>>
diff mbox

Patch

From 4229b304b98dff0190922ddd5270a1389321313b Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sun, 19 Jun 2016 12:47:45 -0700
Subject: [PATCH] Convert V1TImode register to TImode in debug insn

TImode register referenced in debug insn can be converted to V1TImode
by scalar to vector optimization.  We need to convert a debug insn if
it references TImode register which will be converted to V1TImode.

gcc/

	PR target/71549
	* config/i386/i386.c (scalar_chain::analyze_register_chain): In
	64-bit mode, add debug insn, which references the register, to
	queue.
	(timode_scalar_chain::convert_insn): Convert V1TImode register
	to SUBREG TImode in debug insn.

gcc/testsuite/

	PR target/71549
	* gcc.target/i386/pr71549.c: New test.
---
 gcc/config/i386/i386.c                  | 32 +++++++++++++++++++++++++++++++-
 gcc/testsuite/gcc.target/i386/pr71549.c | 24 ++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr71549.c

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 56a5b9c..cea632c 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -3229,8 +3229,22 @@  scalar_chain::analyze_register_chain (bitmap candidates, df_ref ref)
   for (chain = DF_REF_CHAIN (ref); chain; chain = chain->next)
     {
       unsigned uid = DF_REF_INSN_UID (chain->ref);
+      rtx_insn *insn = DF_REF_INSN (chain->ref);
 
-      if (!NONDEBUG_INSN_P (DF_REF_INSN (chain->ref)))
+      if (TARGET_64BIT && DEBUG_INSN_P (insn))
+	{
+	  /* In 64-bit mode, if a variable is put in a TImode register,
+	     which may be converted to V1TImode, we need to convert
+	     this debug insn.  */
+	  rtx val = PATTERN (insn);
+	  if (!(GET_MODE (val) == TImode
+		&& GET_CODE (val) == VAR_LOCATION
+		&& REG_P (PAT_VAR_LOCATION_LOC (val))))
+	    gcc_unreachable ();
+	  add_to_queue (uid);
+	  continue;
+	}
+      else if (!NONDEBUG_INSN_P (insn))
 	continue;
 
       if (!DF_REF_REG_MEM_P (chain->ref))
@@ -3795,6 +3809,22 @@  dimode_scalar_chain::convert_insn (rtx_insn *insn)
 void
 timode_scalar_chain::convert_insn (rtx_insn *insn)
 {
+  if (DEBUG_INSN_P (insn))
+    {
+      /* It must be a debug insn with a TImode variable in register.  */
+      rtx val = PATTERN (insn);
+      gcc_assert (GET_MODE (val) == TImode
+		  && GET_CODE (val) == VAR_LOCATION);
+      rtx loc = PAT_VAR_LOCATION_LOC (val);
+      gcc_assert (REG_P (loc)
+		  && GET_MODE (loc) == V1TImode);
+      /* Convert V1TImode register, which has been updated by a SET
+	 insn before, to SUBREG TImode.  */
+      PAT_VAR_LOCATION_LOC (val) = gen_rtx_SUBREG (TImode, loc, 0);
+      df_insn_rescan (insn);
+      return;
+    }
+
   rtx def_set = single_set (insn);
   rtx src = SET_SRC (def_set);
   rtx dst = SET_DEST (def_set);
diff --git a/gcc/testsuite/gcc.target/i386/pr71549.c b/gcc/testsuite/gcc.target/i386/pr71549.c
new file mode 100644
index 0000000..8aac891
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr71549.c
@@ -0,0 +1,24 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -g" } */
+
+struct S1
+{
+  int f0;
+  int f1;
+  int f2;
+  int:4;
+} a, b;
+
+void
+fn1 (struct S1 p1)
+{
+  a = p1;
+  int c = p1.f0;
+}
+
+int
+main ()
+{
+  fn1 (b);
+  return 0;
+}
-- 
2.5.5