[10/11] TI C6X binutils port: gas/

[DJ Delorie]

+#ifdef TC_IA64
+#define PREDICATE_START '('
+#define PREDICATE_END ')'
+#endif
+#ifdef TC_TIC6X
+#define PREDICATE_START '['
+#define PREDICATE_END ']'
+#endif

Should we consider moving these to the tc-*.h files?  We'd need a
slightly more global-aware name for them, though.

+#ifndef TC_TIC6X
+	      /* For TI C6X, we keep these spaces as they may separate
+		 functional unit specifiers from operands.  */
 	      if (scrub_m68k_mri)
+#endif

Likewise.

+#define TRUNC(X)	((valueT) (X) & 0xffffffff)
+#define SEXT(X)		((TRUNC (X) ^ 0x80000000) - 0x80000000)

These constants need 'U' suffixes (and perhaps UL but I don't think so
for the hosts we care about) to avoid compile-time warnings.  Please
make sure you haven't disabled -Wall/-Werror during your testing.

Also, same comment about commenting abort()s - make sure none of them
can be triggered by user input, comment each with why they might happen,
etc.

+	this_opc_fu_ok = FALSE;;

Extra semicolons.

+  if (num_matching_opcodes == 0)
+    abort ();

Do you really want an abort() here?  There are no cases where a group of
same-named opcodes don't cover 100% of the cases the syntax allows for?

+    case BFD_RELOC_NONE:
+      /* Force output to the object file.  */
+      fixP->fx_done = 0;
+      break;

What are you expecting in the object file?

+	  newval &= 0xff80007f;

You need a U suffix here, and a few other similar places.

+	  newval &= 0xff8000ff;
+	  newval |= ((value >> 1) & 0x7fff) << 8;

There are nine of these.  You might consider making this construct a
macro, if the shift patterns are predictable/computable enough.

+	  /* Handle a constant found to be such at fix time the same
+	     as one found to be such on the first assembly pass.  */
+	  if (fixP->tc_fix_data.fix_adda && fixP->fx_done)
+	    value <<= 1;

I read this a bunch of times, I think it means "if we saw a constant the
first time around, and did something to it, and we see the same constant
this time around, do the same thing to it."  Yes?  If so, I suggest two
commas (or a rewrite ;) :

+	  /* Handle a constant, found to be such at fix time, the same
+	     as one found to be such on the first assembly pass.  */

+  /* Whether this fix was for an ADDA instruction.  If so, a constant
+     resulting from resolving the fix should be implicitly shifted
+     left (it represents a value to be encoded literally in the
+     instruction, whereas a non-constant represents a DP-relative
+     value counting in the appropriate units).  */

This explains the comment that confused me above, but makes me think the
other comment ("Handle a constant...") needs rewording.

+#define md_after_parse_args() tic6x_after_parse_args ()
+extern void tic6x_after_parse_args (void);
+#define md_cleanup() tic6x_cleanup ()
+extern void tic6x_cleanup (void);
+#define md_cons_align(n) tic6x_cons_align (n)
+extern void tic6x_cons_align (int n);
+#define md_parse_name(name, exprP, mode, nextcharP)	\
+  tic6x_parse_name (name, exprP, mode, nextcharP)
+extern int tic6x_parse_name (const char *name, expressionS *exprP,
+			     enum expr_mode mode, char *nextchar);
+#define md_start_line_hook() tic6x_start_line_hook ()
+extern void tic6x_start_line_hook (void);
+#define TC_CONS_FIX_NEW(frag, where, size, exp)	\
+  tic6x_cons_fix_new (frag, where, size, exp)
+extern void tic6x_cons_fix_new (fragS *frag, int where, int size,
+				expressionS *exp);
+#define tc_frob_label(sym) tic6x_frob_label (sym)
+extern void tic6x_frob_label (symbolS *sym);
+#define tc_unrecognized_line(c) tic6x_unrecognized_line (c)
+extern int tic6x_unrecognized_line (int c);

Perhaps some whitespace here?

+The following values of @var{arch} are accepted: @code{c62x},
+@code{c64x}, @code{c64x+}, @code{c67x}, @code{c67x+}, @code{c674x}.

We allow target triplets like tic6x-elf, should we allow arches like
-march=tic64x ?

+Enable or disable the optional C64x+ atomic operation instructions.

You should describe the interactions between -march and -matomic, and
the default if no -m[no-]atomic is given.


Your gas port doesn't yet support the compact instructions.  Is this
planned for the future, or perhaps as some form of relaxation?