Hi Xingxing,

On 19/12/14 11:01, Xingxing Pan wrote:
+/* Return true if vector element size is byte. */
Minor nit: two spaces after full stop and before */ Same in other places in the patch.

+bool
+marvell_whitney_vector_element_size_is_byte (rtx insn)
+{
+  if (GET_CODE (PATTERN (insn)) == SET)
+    {
+      if ((GET_MODE (SET_DEST (PATTERN (insn))) == V8QImode) ||
+          (GET_MODE (SET_DEST (PATTERN (insn))) == V16QImode))
+       return true;
+    }
+
+  return false;
+}

I see this is called from inside marvell-whitney.md. It seems to me that this function takes RTX insns. Can the type of this be strengthened to rtx_insn * ? Also, this should be refactored and written a bit more generally by checking for VECTOR_MODE_P and then GET_MODE_INNER for QImode, saving you the trouble of enumerating the different vector QI modes.


+
+/* Return true if INSN has shift operation but is not a shift insn. */
+bool
+marvell_whitney_non_shift_with_shift_operand (rtx insn)

Similar comment. Can this be strengthened to rtx_insn * ?

Thanks,
Kyrill
+{
+  rtx pat = PATTERN (insn);
+
+  if (GET_CODE (pat) != SET)
+    return false;
+
+  /* Is not a shift insn. */
+  rtx rvalue = SET_SRC (pat);
+  RTX_CODE code = GET_CODE (rvalue);
+  if (code == ASHIFT || code == ASHIFTRT
+      || code == LSHIFTRT || code == ROTATERT)
+    return false;
+
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, rvalue, ALL)
+    {
+      /* Has shift operation. */
+      RTX_CODE code = GET_CODE (*iter);
+      if (code == ASHIFT || code == ASHIFTRT
+          || code == LSHIFTRT || code == ROTATERT)
+        return true;
+    }
+
+  return false;
+}


Reply via email to