diff mbox series

[20/31] VAX: Fix predicates and constraints for EXTV/EXTZV/INSV insns

Message ID alpine.LFD.2.21.2011200255040.656242@eddie.linux-mips.org
State Accepted
Headers show
Series VAX: Bring the port up to date (yes, MODE_CC conversion is included) | expand

Commit Message

Maciej W. Rozycki Nov. 20, 2020, 3:35 a.m. UTC 
It makes no sense for insn operand predicates, as long as they accept a
register operand, to be more restrictive than the set of the associated
constraints, because expand will choose the insn based on the relevant
operand being a pseudo register then and reload keep it happily as a
memory reference if a constraint permits it.  So the restriction posed
by such a predicate will be happily ignored, and moreover if a splitter
is added, such as required for MODE_CC support, the new instructions
will reject the original operands supplied, causing an ICE.  An actual
example will be given with a subsequent change.

Remove such inconsistencies we have with the EXTV/EXTZV/INSV insns then,
observing that a bitfield located in memory is byte-addressed by the
respective machine instructions and therefore SImode may only be used
with a register or an offsettable memory operand (i.e. not an indexed,
pre-decremented, or post-incremented one), which has already been taken
into account with the constraints currently used, except for `*insv_2'.
The QI machine mode may be used for the bitfield location with any kind
of memory operand, but we got the constraint wrong, although harmlessly
in reality, with `*insv'.  Fix that for consistency though.

Also give the insns names, for easier reference here and elsewhere.

	gcc/
	* config/vax/vax.md (*insv_aligned, *extzv_aligned)
	(*extv_aligned, *extv_non_const, *extzv_non_const): Name insns.
	Fix location predicate.
	(*extzv): Name insn.
	(*insv): Likewise.  Fix location constraint.
	(*insv_2): Likewise, and the predicate.
---
 gcc/config/vax/vax.md | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

Comments

Jeff Law Nov. 21, 2020, 5:01 p.m. UTC | #1 
On 11/19/20 8:35 PM, Maciej W. Rozycki wrote:
> It makes no sense for insn operand predicates, as long as they accept a
> register operand, to be more restrictive than the set of the associated
> constraints, because expand will choose the insn based on the relevant
> operand being a pseudo register then and reload keep it happily as a
> memory reference if a constraint permits it.  So the restriction posed
> by such a predicate will be happily ignored, and moreover if a splitter
> is added, such as required for MODE_CC support, the new instructions
> will reject the original operands supplied, causing an ICE.  An actual
> example will be given with a subsequent change.
>
> Remove such inconsistencies we have with the EXTV/EXTZV/INSV insns then,
> observing that a bitfield located in memory is byte-addressed by the
> respective machine instructions and therefore SImode may only be used
> with a register or an offsettable memory operand (i.e. not an indexed,
> pre-decremented, or post-incremented one), which has already been taken
> into account with the constraints currently used, except for `*insv_2'.
> The QI machine mode may be used for the bitfield location with any kind
> of memory operand, but we got the constraint wrong, although harmlessly
> in reality, with `*insv'.  Fix that for consistency though.
>
> Also give the insns names, for easier reference here and elsewhere.
>
> 	gcc/
> 	* config/vax/vax.md (*insv_aligned, *extzv_aligned)
> 	(*extv_aligned, *extv_non_const, *extzv_non_const): Name insns.
> 	Fix location predicate.
> 	(*extzv): Name insn.
> 	(*insv): Likewise.  Fix location constraint.
> 	(*insv_2): Likewise, and the predicate.
OK.
jeff
diff mbox series

Patch

diff --git a/gcc/config/vax/vax.md b/gcc/config/vax/vax.md
index f8e1c2eb02b..de90848a600 100644
--- a/gcc/config/vax/vax.md
+++ b/gcc/config/vax/vax.md
@@ -757,8 +757,8 @@  (define_insn ""
 ;; These handle aligned 8-bit and 16-bit fields,
 ;; which can usually be done with move instructions.
 
-(define_insn ""
-  [(set (zero_extract:SI (match_operand:SI 0 "register_operand" "+ro")
+(define_insn "*insv_aligned"
+  [(set (zero_extract:SI (match_operand:SI 0 "nonimmediate_operand" "+ro")
 			 (match_operand:QI 1 "const_int_operand" "n")
 			 (match_operand:SI 2 "const_int_operand" "n"))
 	(match_operand:SI 3 "general_operand" "g"))]
@@ -786,9 +786,9 @@  (define_insn ""
   return \"movw %3,%0\";
 }")
 
-(define_insn ""
+(define_insn "*extzv_aligned"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=&g")
-	(zero_extract:SI (match_operand:SI 1 "register_operand" "ro")
+	(zero_extract:SI (match_operand:SI 1 "nonimmediate_operand" "ro")
 			 (match_operand:QI 2 "const_int_operand" "n")
 			 (match_operand:SI 3 "const_int_operand" "n")))]
   "(INTVAL (operands[2]) == 8 || INTVAL (operands[2]) == 16)
@@ -814,9 +814,9 @@  (define_insn ""
   return \"movzwl %1,%0\";
 }")
 
-(define_insn ""
+(define_insn "*extv_aligned"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
-	(sign_extract:SI (match_operand:SI 1 "register_operand" "ro")
+	(sign_extract:SI (match_operand:SI 1 "nonimmediate_operand" "ro")
 			 (match_operand:QI 2 "const_int_operand" "n")
 			 (match_operand:SI 3 "const_int_operand" "n")))]
   "(INTVAL (operands[2]) == 8 || INTVAL (operands[2]) == 16)
@@ -842,7 +842,7 @@  (define_insn ""
   return \"cvtwl %1,%0\";
 }")
 
-;; Register-only SImode cases of bit-field insns.
+;; Register and non-offsettable-memory SImode cases of bit-field insns.
 
 (define_insn ""
   [(set (cc0)
@@ -869,9 +869,9 @@  (define_insn ""
 ;; by a bicl or sign extension.  Because we might end up choosing ext[z]v
 ;; anyway, we can't allow immediate values for the primary source operand.
 
-(define_insn ""
+(define_insn "*extv_non_const"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
-	(sign_extract:SI (match_operand:SI 1 "register_operand" "ro")
+	(sign_extract:SI (match_operand:SI 1 "nonimmediate_operand" "ro")
 			 (match_operand:QI 2 "general_operand" "g")
 			 (match_operand:SI 3 "general_operand" "nrmT")))]
   ""
@@ -886,9 +886,9 @@  (define_insn ""
   return \"rotl %R3,%1,%0\;cvtwl %0,%0\";
 }")
 
-(define_insn ""
+(define_insn "*extzv_non_const"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
-	(zero_extract:SI (match_operand:SI 1 "register_operand" "ro")
+	(zero_extract:SI (match_operand:SI 1 "nonimmediate_operand" "ro")
 			 (match_operand:QI 2 "general_operand" "g")
 			 (match_operand:SI 3 "general_operand" "nrmT")))]
   ""
@@ -962,7 +962,7 @@  (define_expand "extzv"
   ""
   "")
 
-(define_insn ""
+(define_insn "*extzv"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
 	(zero_extract:SI (match_operand:QI 1 "memory_operand" "m")
 			 (match_operand:QI 2 "general_operand" "g")
@@ -1015,8 +1015,8 @@  (define_expand "insv"
   ""
   "")
 
-(define_insn ""
-  [(set (zero_extract:SI (match_operand:QI 0 "memory_operand" "+g")
+(define_insn "*insv"
+  [(set (zero_extract:SI (match_operand:QI 0 "memory_operand" "+m")
 			 (match_operand:QI 1 "general_operand" "g")
 			 (match_operand:SI 2 "general_operand" "nrmT"))
 	(match_operand:SI 3 "general_operand" "nrmT"))]
@@ -1046,8 +1046,8 @@  (define_insn ""
   return \"insv %3,%2,%1,%0\";
 }")
 
-(define_insn ""
-  [(set (zero_extract:SI (match_operand:SI 0 "register_operand" "+r")
+(define_insn "*insv_2"
+  [(set (zero_extract:SI (match_operand:SI 0 "nonimmediate_operand" "+ro")
 			 (match_operand:QI 1 "general_operand" "g")
 			 (match_operand:SI 2 "general_operand" "nrmT"))
 	(match_operand:SI 3 "general_operand" "nrmT"))]