This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [10/11] TI C6X binutils port: gas/
+#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?