This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] PR target/70155: Use SSE for TImode load/store
- From: Ilya Enkovich <enkovich dot gnu at gmail dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: Uros Bizjak <ubizjak at gmail dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Richard Biener <rguenther at suse dot de>
- Date: Mon, 25 Apr 2016 19:13:58 +0300
- Subject: Re: [PATCH] PR target/70155: Use SSE for TImode load/store
- Authentication-results: sourceware.org; auth=none
- References: <20160425125145 dot GA5326 at intel dot com> <CAFULd4ZFMuMR0UVtxYL6teuO-CABSJv6QbqXhkU5DX5F_aGdYQ at mail dot gmail dot com> <CAMe9rOrzOZYGSwq9g9aj3xSjSHnGuBJh_rmFTai0V7eUPkaSTw at mail dot gmail dot com> <CAFULd4Z54Gv2GQcjz=qfmd0PSaON44zz-h7SWBvKLhTzgOOzKg at mail dot gmail dot com> <CAMe9rOqMxQruLKDakrO6s=gcUmg-QQJdABEUbOzu6=cKgMAOog at mail dot gmail dot com>
2016-04-25 18:27 GMT+03:00 H.J. Lu <hjl.tools@gmail.com>:
>
> Ilya, can you take a look?
>
> Thanks.
>
> --
> H.J.
Hi,
Algorithmic part of the patch looks OK to me except the following piece of code.
+/* Check REF's chain to add new insns into a queue
+ and find registers requiring conversion. */
Comment is wrong because you don't have any conversions required for
your candidates.
+
+void
+scalar_chain_64::analyze_register_chain (bitmap candidates, df_ref ref)
+{
+ df_link *chain;
+
+ gcc_assert (bitmap_bit_p (insns, DF_REF_INSN_UID (ref))
+ || bitmap_bit_p (candidates, DF_REF_INSN_UID (ref)));
+ add_to_queue (DF_REF_INSN_UID (ref));
+
+ for (chain = DF_REF_CHAIN (ref); chain; chain = chain->next)
+ {
+ unsigned uid = DF_REF_INSN_UID (chain->ref);
+
+ if (!NONDEBUG_INSN_P (DF_REF_INSN (chain->ref)))
+ continue;
+
+ if (!DF_REF_REG_MEM_P (chain->ref))
+ continue;
I believe here you wrongly jump to the next ref intead of actually adding it
to a queue. You may just use
gcc_assert (!DF_REF_REG_MEM_P (chain->ref));
because you should'n have a candidate used in address operand.
+
+ if (bitmap_bit_p (insns, uid))
+ continue;
+
+ if (bitmap_bit_p (candidates, uid))
+ add_to_queue (uid);
Probably gcc_assert (bitmap_bit_p (candidates, uid)) since no uses and defs
out of candidates list are allowed?
+ }
+}
Thanks,
Ilya