Re: Silly gcc bugs

["Maciej W. Rozycki" <macro%linux-mips.org@localhost>]

On Sun, 14 Feb 2016, Maciej W. Rozycki wrote:

>  I've checked the generated assembly and I think it is correct although I 
> can't easily verify it at the run time right now, so I'll appreciate if 
> you try this change and let me know if the code that failed to build also 
> works correctly.
> 
>  If it works for your case, then it would be nice to put it through full 
> GCC regression testing too, however doing that in a cross-compilation 
> environment is tricky and I'm not set up for native testing with the 
> VAX/Linux port, it's simply too immature.  So if someone steps in and does 
> that, then I'll appreciate it -- in a native environment it's as easy as 
> running `make check' (or maybe `make -C gcc check' to exclude libraries 
> from being tested in return for a quicker run) in your build tree.  

 Martin was kind enough to put it through testing (thanks!) and it choke 
on the offset being output by GCC from `*addsi4_mova' as an immediate, 
that is with `$' prepended.  That didn't trigger with my previous testing 
as gchecksum.s didn't include any output generated from the `*addsi4_mova' 
insn, however it can be easily reproduced with code like (the asm only 
needed to force data into registers in the most compact way):

int add(int x, int y)
{
	asm("" : "+r" (x), "+r" (y));

	return x + 4 * y + 8;
}

 Here's a patch update which corrects it, by adding a new operand code 
that prints a constant with the `$' prefix omitted.  There's no existing 
code that could serve this purpose (`x' is close, present in HEAD only and 
not in 4.1, however I'd keep it decimal for consistency), so I added a new 
one, placing it such as not to cause a conflict with HEAD.  TBH I think it 
would be nice if these operand codes were actually documented like with 
the other ports. ;)

 I hope this was the last issue, but please do let me know even if not.

 Interestingly enough if `4' in the function above is changed to `2', then 
`*addsi4_mova' is not matched for some reason and instead of a single 
MOVAW instruction a longer sequence using ADDL2 followed by MOVAB is 
produced.  The lengths of MOVAW and MOVAB here are both the same and both 
use the indexed addressing mode, so I can't see how the longer sequence 
can have any benefit, or indeed no loss.  I smell some costs may need 
tweaking, and maybe an insn for `ashift' rather than `mult' added too.  
Just a side note though.

2016-02-14  Maciej W. Rozycki  <macro%linux-mips.org@localhost>

	gcc/
	* config/vax/vax.c (print_operand): Add the `o' operand code.
	* config/vax/predicates.md (const_248_operand): New predicate.
	* config/vax/vax.md (*addsi3_mova, *addsi4_mova): New insns.

  Maciej

gcc-4.1.2-vax-mova.patch
Index: gcc-4.1.2/gcc/config/vax/predicates.md
===================================================================
--- gcc-4.1.2.orig/gcc/config/vax/predicates.md
+++ gcc-4.1.2/gcc/config/vax/predicates.md
@@ -102,3 +102,11 @@
    (and (match_code "const_int,const_double,subreg,reg,mem")
 	(and (match_operand:DI 0 "general_operand" "")
 	     (not (match_operand:DI 0 "illegal_addsub_di_memory_operand")))))
+
+;; Match 2, 4, or 8.  Used for mova multiplicands.
+(define_predicate "const_248_operand"
+   (match_code "const_int")
+{
+  HOST_WIDE_INT i = INTVAL (op);
+  return i == 2 || i == 4 || i == 8;
+})
Index: gcc-4.1.2/gcc/config/vax/vax.c
===================================================================
--- gcc-4.1.2.orig/gcc/config/vax/vax.c
+++ gcc-4.1.2/gcc/config/vax/vax.c
@@ -425,6 +425,8 @@ print_operand (FILE *file, rtx x, int co
     fputs (REGISTER_PREFIX, file);
   else if (code == 'C')
     fputs (rev_cond_name (x), file);
+  else if (code == 'o' && CONST_INT_P (x))
+    fprintf (file, HOST_WIDE_INT_PRINT_DEC, INTVAL (x));
   else if (code == 'D' && CONST_INT_P (x) && INTVAL (x) < 0)
     fprintf (file, "$" NEG_HWI_PRINT_HEX16, INTVAL (x));
   else if (code == 'P' && CONST_INT_P (x))
Index: gcc-4.1.2/gcc/config/vax/vax.md
===================================================================
--- gcc-4.1.2.orig/gcc/config/vax/vax.md
+++ gcc-4.1.2/gcc/config/vax/vax.md
@@ -405,6 +405,54 @@
 		 (match_operand:DI 2 "general_operand" "Fsro,Fs")))]
   "!TARGET_QMATH"
   "* return vax_output_int_add (insn, operands, DImode);")
+
+;; Scaled additions using mova, needed for virtual register instantiation.
+(define_insn "*addsi3_mova"
+  [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
+	(plus:SI (mult:SI (match_operand:SI 1 "register_operand" "r")
+			  (match_operand 2 "const_248_operand" "n"))
+		 (match_operand:SI 3 "register_operand" "r")))]
+  ""
+  "*
+{
+  int i = INTVAL (operands[2]);
+
+  switch (i)
+    {
+    case 2:
+      return \"movaw (%3)[%1],%0\";
+    case 4:
+      return \"moval (%3)[%1],%0\";
+    case 8:
+      return \"movaq (%3)[%1],%0\";
+    default:
+      gcc_unreachable ();
+    }
+}")
+
+(define_insn "*addsi4_mova"
+  [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
+	(plus:SI (plus:SI (mult:SI (match_operand:SI 1 "register_operand" "r")
+				   (match_operand 2 "const_248_operand" "n"))
+			  (match_operand:SI 3 "register_operand" "r"))
+		 (match_operand:SI 4 "immediate_operand" "i")))]
+  ""
+  "*
+{
+  int i = INTVAL (operands[2]);
+
+  switch (i)
+    {
+    case 2:
+      return \"movaw %o4(%3)[%1],%0\";
+    case 4:
+      return \"moval %o4(%3)[%1],%0\";
+    case 8:
+      return \"movaq %o4(%3)[%1],%0\";
+    default:
+      gcc_unreachable ();
+    }
+}")
 
 ;;- All kinds of subtract instructions.