Re: [PATCH 1/2] arm: Add define_attr to to create a mapping between MVE predicated and unpredicated insns

[Richard Earnshaw <Richard.Earnshaw@foss.arm.com>]

On 06/11/2023 11:20, Stamatis Markianos-Wright wrote:
> Patch has already been approved at:
> 
> https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630326.html
> 
> 
> ... But I'm sending this again for archiving on the list after rebasing

A couple of minor nits:

1)

+#define MVE_VPT_PREDICABLE_INSN_P(INSN)					\
+  (recog_memoized (INSN) >= 0						\
+  && get_attr_mve_unpredicated_insn (INSN) != 0)			\

I think it's better to write "!= CODE_FOR_nothing".

+(define_attr "mve_unpredicated_insn" "" (const_int 0))
+

And the default value here should similarly be 'symbol_ref 
"CODE_FOR_nothing"'.

So that the style matches the symbol refs elsewhere.


2)
+(define_insn "*predicated_doloop_end_internal"
+  [(set (pc)
+	(if_then_else
+	   (ge (plus:SI (reg:SI LR_REGNUM)
+			(match_operand:SI 0 "const_int_operand" ""))
+		(const_int 0))
+	 (label_ref (match_operand 1 "" ""))
+	 (pc)))
+   (set (reg:SI LR_REGNUM)
+	(plus:SI (reg:SI LR_REGNUM) (match_dup 0)))
+   (clobber (reg:CC CC_REGNUM))]
+  "TARGET_32BIT && TARGET_HAVE_LOB && TARGET_HAVE_MVE && TARGET_THUMB2"

TARGET_THUMB2 => TARGET_32BIT, so the first test is redundant.  In fact, 
given that TARGET_HAVE_LOB => armv8.1-m.main => thumb2, why do we need 
either?

So
	TARGET_HAVE_LOB && TARGET_HAVE_MVE
should be sufficient.


+(define_insn "dlstp<mode1>_insn"
+  [
+    (set (reg:SI LR_REGNUM)
+	 (unspec:SI [(match_operand:SI 0 "s_register_operand" "r")]
+	  DLSTP))
+  ]
+  "TARGET_32BIT && TARGET_HAVE_LOB && TARGET_HAVE_MVE && TARGET_THUMB2"

Same here.

Otherwise, OK.

R.