On Mon, Jun 20, 2016 at 7:30 AM, Ilya Enkovich <[email protected]> wrote:
> 2016-06-20 16:39 GMT+03:00 Uros Bizjak <[email protected]>:
>> On Mon, Jun 20, 2016 at 1:55 PM, H.J. Lu <[email protected]> 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.
>>>
--
H.J.
From 4229b304b98dff0190922ddd5270a1389321313b Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <[email protected]>
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
