[PATCH] PR target/70155: Use SSE for TImode load/store

Ilya Enkovich enkovich.gnu@gmail.com
Mon Apr 25 16:14:00 GMT 2016


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



More information about the Gcc-patches mailing list