diff mbox series

[02/31] VAX: Remove `c' operand format specifier overload

Message ID alpine.LFD.2.21.2011200232000.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:34 a.m. UTC
The `c' operand format specifier is handled directly by the middle end
in `output_asm_insn':

   %cN means require operand N to be a constant
      and print the constant expression with no punctuation.

however it resorts to the target for constants that are not valid
addresses:

	    else if (letter == 'c')
	      {
		if (CONSTANT_ADDRESS_P (operands[opnum]))
		  output_addr_const (asm_out_file, operands[opnum]);
		else
		  output_operand (operands[opnum], 'c');
	      }

The VAX backend expects the fallback never to happen and overloads `c'
with the branch condition code.  This is confusing however and it is not
like we are short of letters, so instead make the branch condition code
use `k', and then for consistency make `K' the reverse branch condition
code format specifier.  This is safe to do as we provide no means to use
a computed branch condition code in user `asm'.

	gcc/
	* config/vax/vax.c (print_operand): Replace `c' and `C' with
	`k' and `K' respectively.
	* config/vax/vax.md (*branch, *branch_reversed): Update
	accordingly.
---
 gcc/config/vax/vax.c  | 4 ++--
 gcc/config/vax/vax.md | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Jeff Law Nov. 20, 2020, 11:16 p.m. UTC | #1
On 11/19/20 8:34 PM, Maciej W. Rozycki wrote:
> The `c' operand format specifier is handled directly by the middle end
> in `output_asm_insn':
>
>    %cN means require operand N to be a constant
>       and print the constant expression with no punctuation.
>
> however it resorts to the target for constants that are not valid
> addresses:
>
> 	    else if (letter == 'c')
> 	      {
> 		if (CONSTANT_ADDRESS_P (operands[opnum]))
> 		  output_addr_const (asm_out_file, operands[opnum]);
> 		else
> 		  output_operand (operands[opnum], 'c');
> 	      }
>
> The VAX backend expects the fallback never to happen and overloads `c'
> with the branch condition code.  This is confusing however and it is not
> like we are short of letters, so instead make the branch condition code
> use `k', and then for consistency make `K' the reverse branch condition
> code format specifier.  This is safe to do as we provide no means to use
> a computed branch condition code in user `asm'.
>
> 	gcc/
> 	* config/vax/vax.c (print_operand): Replace `c' and `C' with
> 	`k' and `K' respectively.
> 	* config/vax/vax.md (*branch, *branch_reversed): Update
> 	accordingly.
OK.

I wonder if other targets have the same problem.  But that doesn't
affect this patch.

Jeff
Segher Boessenkool Nov. 24, 2020, 1:12 a.m. UTC | #2
On Fri, Nov 20, 2020 at 03:34:16AM +0000, Maciej W. Rozycki wrote:
> The `c' operand format specifier is handled directly by the middle end
> in `output_asm_insn':
> 
>    %cN means require operand N to be a constant
>       and print the constant expression with no punctuation.
> 
> however it resorts to the target for constants that are not valid
> addresses:
> 
> 	    else if (letter == 'c')
> 	      {
> 		if (CONSTANT_ADDRESS_P (operands[opnum]))
> 		  output_addr_const (asm_out_file, operands[opnum]);
> 		else
> 		  output_operand (operands[opnum], 'c');
> 	      }
> 
> The VAX backend expects the fallback never to happen and overloads `c'
> with the branch condition code.  This is confusing however

There are 16 targets in trunk that overload the 'c' output modifier.
Most do it for something with condition codes (or comparisons).  This is
a well-established convention, confusing or not :-)

(I have nothing against the patch of course.)


Segher
diff mbox series

Patch

diff --git a/gcc/config/vax/vax.c b/gcc/config/vax/vax.c
index da4e6cb1745..0b3b76ed6da 100644
--- a/gcc/config/vax/vax.c
+++ b/gcc/config/vax/vax.c
@@ -509,9 +509,9 @@  print_operand (FILE *file, rtx x, int code)
     fputc (ASM_DOUBLE_CHAR, file);
   else if (code == '|')
     fputs (REGISTER_PREFIX, file);
-  else if (code == 'c')
+  else if (code == 'k')
     fputs (cond_name (x), file);
-  else if (code == 'C')
+  else if (code == 'K')
     fputs (rev_cond_name (x), file);
   else if (code == 'D' && CONST_INT_P (x) && INTVAL (x) < 0)
     fprintf (file, "$" NEG_HWI_PRINT_HEX16, INTVAL (x));
diff --git a/gcc/config/vax/vax.md b/gcc/config/vax/vax.md
index 4897ce44505..e3018a0ee06 100644
--- a/gcc/config/vax/vax.md
+++ b/gcc/config/vax/vax.md
@@ -1111,7 +1111,7 @@  (define_insn "*branch"
 		      (label_ref (match_operand 1 "" ""))
 		      (pc)))]
   ""
-  "j%c0 %l1")
+  "j%k0 %l1")
 
 ;; Recognize reversed jumps.
 (define_insn "*branch_reversed"
@@ -1122,7 +1122,7 @@  (define_insn "*branch_reversed"
 		      (pc)
 		      (label_ref (match_operand 1 "" ""))))]
   ""
-  "j%C0 %l1") ; %C0 negates condition
+  "j%K0 %l1") ; %K0 negates condition
 
 ;; Recognize jbs, jlbs, jbc and jlbc instructions.  Note that the operand
 ;; of jlbs and jlbc insns are SImode in the hardware.  However, if it is