On 07/14/2013 09:54 AM, Chung-Lin Tang wrote:
> Hi, the last ping of the Nios II patches was:
> http://gcc.gnu.org/ml/gcc-patches/2013-06/msg01416.html
>
> After assessing the state, we feel it would be better to post a
> re-submission of the newest patches.
Since this hasn't attracted attention for months I'll have a go at a
review. I was not involved in this project and haven't seen this code
before.
> @@ -4196,6 +4203,7 @@ esac
> # version to the per-target configury.
> case "$cpu_type" in
> alpha | arm | avr | bfin | cris | i386 | m32c | m68k | microblaze | mips \
> + | nios2 \
> | pa | rs6000 | score | sparc | spu | tilegx | tilepro | xstormy16 |
> xtensa)
> insn="nop"
Could maybe format this nicer to fill up all but the last line.
> +;; These are documented for use in inline asm.
> +(define_register_constraint "D00" "D00_REG" "Hard register 0.")
> +(define_register_constraint "D01" "D01_REG" "Hard register 1.")
> +(define_register_constraint "D02" "D02_REG" "Hard register 2.")
[...]
This doesn't strike me as a good idea. To really make this work for
cases where people write things like "D28D13" you'd have to provide all
the union classes as well which is obviously infeasible. There are
alternative mechanisms to get assembly to use the registers you want,
and I'll note that libgcc seems to be using those instead of these
constraints.
This probably also slows down the compiler, and it might be worth an
experiment to see whether deleting these has any effect on register
allocation.
> +;; We use the following constraint letters for constants
> +;;
> +;; I: -32768 to -32767
> +;; J: 0 to 65535
> +(define_insn "movhi_internal"
> + [(set (match_operand:HI 0 "nonimmediate_operand" "=m, r,r, r,r")
> + (match_operand:HI 1 "general_operand" "rM,m,rM,I,J"))]
The J alternative is unnecessary. The range 32768...65535 doesn't give a
valid HImode constant, the RTL representation of integer constants
always sign extends. Probably you can just use "i" here and in
movqi_internal.
> +(define_insn "movsi_internal"
> + [(set (match_operand:SI 0 "nonimmediate_operand" "=m, r,r, r,r,r,r,r")
> + (match_operand:SI 1 "general_operand" "rM,m,rM,I,J,K,S,i"))]
Not required, but as an experiment you might want to reorder the
alternatives and sprinkle extra "r"s to get optional reloads and see
whether that improves code generation. You'd have
(match_operand:SI 0 "nonimmediate_operand" "=r,rm,r ,r ,r ,r ,r ,r")
(match_operand:SI 1 "general_operand" "rM,rM,rm,rI,rJ,rK,rS,ri"))]
with the register-register move first. It might not make a difference.
> +;; Split patterns for register alternative cases.
> +(define_split
> + [(set (match_operand:SI 0 "register_operand" "")
> + (sign_extend:SI (match_operand:HI 1 "register_operand" "")))]
Could merge into a define_insn_and_split?
> + "reload_completed"
> + [(set (match_dup 0)
> + (and:SI (match_dup 1) (const_int 65535)))
> + (set (match_dup 0)
> + (xor:SI (match_dup 0) (const_int 32768)))
> + (set (match_dup 0)
> + (plus:SI (match_dup 0) (const_int -32768)))]
> + "operands[1] = gen_lowpart (SImode, operands[1]);")
Is this really faster than two shifts (either as expander or splitter)?
> +;; Arithmetic Operations
> +
> +(define_insn "addsi3"
> + [(set (match_operand:SI 0 "register_operand" "=r")
> + (plus:SI (match_operand:SI 1 "register_operand" "%r")
> + (match_operand:SI 2 "arith_operand" "rIT")))]
> + ""
> + "add%i2\\t%0, %1, %z2"
> + [(set_attr "type" "alu")])
[...]
> +(define_insn "mulsi3"
> + [(set (match_operand:SI 0 "register_operand" "=r")
> + (mult:SI (match_operand:SI 1 "register_operand" "%r")
> + (match_operand:SI 2 "arith_operand" "rI")))]
> + "TARGET_HAS_MUL"
> + "mul%i2\\t%0, %1, %z2"
> + [(set_attr "type" "mul")])
There seems to be a slight mismatch here between arith_operand and the
constraints. Unlike in addsi3, "T" (which calls nios2_unspec_reloc_p)
isn't allowed here and in the other uses of arith_operand. That suggests
that arith_operand should be split into two different predicates, one
for addsi3 and one for all the other uses.
> +(define_expand "divsi3"
> + [(set (match_operand:SI 0 "register_operand" "=r")
> + (div:SI (match_operand:SI 1 "register_operand" "r")
> + (match_operand:SI 2 "register_operand" "r")))]
> + ""
> +{
> + if (!TARGET_HAS_DIV)
> + {
> + if (!TARGET_FAST_SW_DIV)
> + FAIL;
> + else
> + {
> + if (nios2_emit_expensive_div (operands, SImode))
> + DONE;
> + }
> + }
> +})
Shouldn't this FAIL if !nios2_emit_expensive_div (ok so that function
never returns 0 but still...)?
> +;; Integer logical Operations
> +
> +(define_code_iterator LOGICAL [and ior xor])
> +(define_code_attr logical_asm [(and "and") (ior "or") (xor "xor")])
> +
> +(define_insn "<code>si3"
> + [(set (match_operand:SI 0 "register_operand" "=r,r,r")
> + (LOGICAL:SI (match_operand:SI 1 "register_operand" "%r,r,r")
> + (match_operand:SI 2 "logical_operand" "rM,J,K")))]
> + ""
> + "@
> + <logical_asm>\\t%0, %1, %z2
> + <logical_asm>%i2\\t%0, %1, %2
> + <logical_asm>h%i2\\t%0, %1, %U2"
> + [(set_attr "type" "alu")])
> +
> +(define_insn "*norsi3"
> + [(set (match_operand:SI 0 "register_operand" "=r")
> + (and:SI (not:SI (match_operand:SI 1 "register_operand" "%r"))
> + (not:SI (match_operand:SI 2 "reg_or_0_operand" "rM"))))]
> + ""
> + "nor\\t%0, %1, %z2"
> + [(set_attr "type" "alu")])
M constraints (for const0_rtx) and reg_or_0 seem unnecessary, no such
RTL should make it to this point.
> +;; Floating point instructions
Ok, all this stuff is scary, but it seems to be dictated by the hardware.
> + "* return nios2_fpu_insn_asm (n2fpu_f<fop3><f>);"
Minor point - that's an older form, personally I prefer
{
return nios2_fpu_insn_asm (n2fpu_f<fop3><f>);
}
over C-in-a-string, but I'm not sure there's any sort of consensus that
we shouldn't be doing the former.
> +(define_insn "return"
> + [(return)]
> + "nios2_can_use_return_insn ()"
> + "ret"
> +)
Could try to also define "simple_return" to get shrink-wrapping. Move
closing paren to previous line, here and elsewhere. Define a type?
> +(define_expand "call"
> + [(parallel [(call (match_operand 0 "" "")
> + (match_operand 1 "" ""))
> + (clobber (reg:SI 31))])]
> + ""
> + "nios2_adjust_call_address (&operands[0]);"
> +)
Weird. Why isn't r31 (return address) in CALL_USED_REGS? Should do that
and lose the clobber unless there's a very good reason.
> + [(set_attr "type" "control,control")])
I think just "control" would be better for the type. Multiple instances.
> +;; Integer comparisons
> +
> +(define_code_iterator EQNE [eq ne])
> +(define_insn "nios2_cmp<code>"
> + [(set (match_operand:SI 0 "register_operand" "=r")
> + (EQNE:SI (match_operand:SI 1 "reg_or_0_operand" "%rM")
> + (match_operand:SI 2 "arith_operand" "rI")))]
> + ""
> + "cmp<code>%i2\\t%0, %z1, %z2"
> + [(set_attr "type" "alu")])
Once again, using reg_or_0 and "M" seems pointless.
> +(define_insn "trap"
> + [(unspec_volatile [(const_int 0)] UNSPECV_TRAP)]
There's a trap_if rtl code.
> +(define_insn "stack_overflow_detect_and_trap"
> + [(unspec_volatile [(const_int 0)] UNSPECV_STACK_OVERFLOW_DETECT_AND_TRAP)]
Likewise. Might even be able to represent the condition.
> +/* Local prototypes. */
I'd much prefer not to have any of those. Achieve this by putting
> +struct gcc_target targetm = TARGET_INITIALIZER;
along with all the necessary definitions at the end of the file (and
reordering some other functions).
> +/* ??? Might want to redefine TARGET_RETURN_IN_MSB here to handle
> + big-endian case; depends on what ABI we choose. */
This comment needs to be addressed and deleted.
> +/* Stack Layout and Calling Conventions */
Slightly Too Many Caps I Think. Period and two spaces to end comment.
Elsewhere too for these kinds of section comments.
> +#define TOO_BIG_OFFSET(X) ((X) > ((1 << 15) - 1))
Nitpicky, but I don't like the name. I'd rather have a
VALID_ADD_OFFSET_P macro and use its negation.
> +static void
> +save_reg (int regno, unsigned offset)
Strictly speaking these functions should all have comments describing
their args and such.
> +void
> +expand_prologue (void)
This and other globally-visible functions need renaming to have a nios2_
prefix to avoid confusion.
> +{
> + int ix;
> + HOST_WIDE_INT total_frame_size = compute_frame_size ();
> + total_frame_size = compute_frame_size ();
Done twice. Maybe nios2_compute_frame_layout would be a better name
since it does more than compute the size.
> + for (ix = 32; ix--;)
> + if (save_mask & ((unsigned HOST_WIDE_INT)1 << ix))
You shouldn't need to cast to HOST_WIDE_INT, just 1u will do (and
save_mask can be unsigned int).
> + emit_insn
> + (gen_rtx_SET (Pmode, tmp,
> + gen_int_mode (cfun->machine->save_regs_offset,
> + Pmode)));
Shouldn't have a mode on the SET, but really should just call
emit_move_insn. Similar cases elsewhere, search for "gen_rtx_SET (Pmode".
> +nios2_function_profiler (FILE *file, int labelno ATTRIBUTE_UNUSED)
There's now an ARG_UNUSED macro; I'm not sure whether we're supposed to
use that or just omit the arg name now that we're using C++.
> +/* Return true if REGNO should be saved in a prologue. */
"the" prologue?
> +/* Return the bytes needed to compute the frame pointer from the current
> + stack pointer. */
> +static HOST_WIDE_INT
> +compute_frame_size (void)
Needs renaming to at least prepend nios2_; see above. Comment needs
expanding since the function does more than what it says.
> + HOST_WIDE_INT var_size; /* # of var. bytes allocated. */
Not the first time they occur in this file, but I suppose I should
mention that these in-line comments are probably just outside our coding
guidelines.
> + /* Calculate space needed for gp registers. */
> + for (regno = 0; GP_REGNO_P (regno); regno++)
> + if (save_reg_p (regno))
> + {
> + save_mask |= (unsigned HOST_WIDE_INT)1 << regno;
> + save_reg_size += 4;
> + }
Not sure I like GP_REGNO_P (regno) as the loop exit condition here.
> + /* If we call eh_return, we need to save the EH data registers. */
> + if (crtl->calls_eh_return)
Fold this case into save_reg_p maybe? Doesn't matter too much.
> +/* Return nonzero if this function is known to have a null epilogue.
> + This allows the optimizer to omit jumps to jumps if no stack
> + was created. */
> +int
> +nios2_can_use_return_insn (void)
> +{
> + if (!reload_completed)
> + return 0;
> +
> + if (df_regs_ever_live_p (RA_REGNO) || crtl->profile)
> + return 0;
> +
> + if (cfun->machine->initialized)
I think this could be an assert and the if branch unconditional.
> + return cfun->machine->total_size == 0;
> +static void
> +nios2_set_fpu_custom_code (enum n2fpu_code code, int N, bool override_p)
Again, add comments for all these. I haven't looked too hard at the
custom code stuff since it seems unavoidably horrid but doesn't really
affect anything outside it.
Parameter N should be lowercase.
> + if (!strncasecmp (cfg, "60-1", 4))
strcmp, several times. At least judging by the docs allowing "60-1fish"
is unintentional?
> + /* Check for unsupported options. */
> + if (flag_pic && !TARGET_LINUX_ABI)
> + error ("position-independent code requires the Linux ABI");
Use sorry instead?
> +/* Return true if CST is a constant within range of movi/movui/movhi. */
> +static bool
> +nios2_simple_const_p (const_rtx cst)
> +{
> + HOST_WIDE_INT val = INTVAL (cst);
> + return (SMALL_INT (val) || SMALL_INT_UNSIGNED (val) || UPPER16_INT (val));
> +}
Unnecessary parens around return value. Several cases in this file.
> + case CONST_INT:
> + if (INTVAL (x) == 0)
> + {
> + *total = COSTS_N_INSNS (0);
> + return true;
> + }
> + else if (nios2_simple_const_p (x))
> + {
> + *total = COSTS_N_INSNS (2);
> + return true;
Hmm, really? What about insns that accept a constant as operand,
shouldn't it be zero cost for those?
> + case MULT:
> + {
> + *total = COSTS_N_INSNS (1);
> + return false;
> + }
Really?
> +static rtx
> +nios2_call_tls_get_addr (rtx ti)
> +{
> + rtx arg = gen_rtx_REG (Pmode, FIRST_ARG_REGNO);
> + rtx ret = gen_rtx_REG (Pmode, FIRST_RETVAL_REGNO);
> + rtx fn, insn;
> +
> + if (!nios2_tls_symbol)
> + nios2_tls_symbol = init_one_libfunc ("__tls_get_addr");
> +
> + emit_insn (gen_rtx_SET (Pmode, arg, ti));
> + fn = gen_rtx_MEM (QImode, nios2_tls_symbol);
> + insn = emit_call_insn (gen_call_value (ret, fn, const0_rtx));
> + RTL_CONST_CALL_P (insn) = 1;
> + use_reg (&CALL_INSN_FUNCTION_USAGE (insn), ret);
> + use_reg (&CALL_INSN_FUNCTION_USAGE (insn), arg);
> +
> + return ret;
> +}
No special relocs to mark such a call?
> +int
> +nios2_emit_move_sequence (rtx *operands, enum machine_mode mode)
Hmm, I would have guessed that most targets would call this
nios2_expand_move, but grep shows that the two variants are about
equally common.
> +/* Divide Support */
> +
> +/*
> + If -O3 is used, we want to output a table lookup for
Watch comment formatting.
> + divides between small numbers (both num and den >= 0
> + and < 0x10). The overhead of this method in the worse
> + case is 40 bytes in the text section (10 insns) and
"worst case", everywhere.
> + 256 bytes in the data section. Additional divides do
> + not incur additional penalties in the data section.
> +
> + Code speed is improved for small divides by about 5x
> + when using this method in the worse case (~9 cycles
> + vs ~45). And in the worse case divides not within the
> + table are penalized by about 10% (~5 cycles vs ~45).
> + However in the typical case the penalty is not as bad
> + because doing the long divide in only 45 cycles is
> + quite optimistic.
> +
> + ??? It would be nice to have some benchmarks other
> + than Dhrystone to back this up.
Um, ok. This is an optimization for dhrystone? That's... interesting.
> + ??? Ideally I would like the emit libcall block to contain
Lose "emit".
> +void
> +nios2_adjust_call_address (rtx *call_op)
> +{
> + rtx addr;
> + gcc_assert (MEM_P (*call_op));
> + addr = XEXP (*call_op, 0);
> + if (flag_pic && CONSTANT_P (addr))
> + {
> + rtx reg = gen_reg_rtx (Pmode);
> + emit_move_insn (reg, nios2_load_pic_address (addr,
> UNSPEC_PIC_CALL_SYM));
> + XEXP (*call_op, 0) = reg;
> + }
> +}
> +
> +
> +
Some excess newlines here.
> +/* Branches/Compares. */
> +
> +/* Return in *ALT_CODE and *ALT_OP, an alternate equivalent constant
> + comparision, e.g. >= 1 into > 0. */
"comparison".
> +static void
> +nios2_alternate_compare_const (enum rtx_code code, rtx op,
> + enum rtx_code *alt_code, rtx *alt_op,
> + enum machine_mode mode)
> +{
> + HOST_WIDE_INT opval = INTVAL (op);
> + enum rtx_code scode = signed_condition (code);
> + *alt_code = ((code == EQ || code == NE) ? code
> + /* The required conversion between [>,>=] and [<,<=] is captured
> + by a reverse + swap of condition codes. */
> + : reverse_condition (swap_condition (code)));
> + *alt_op = ((scode == LT || scode == GE) ? gen_int_mode (opval - 1, mode)
> + : (scode == LE || scode == GT) ? gen_int_mode (opval + 1, mode)
That's not really equivalent in all cases, right? I don't see anything
dealing with boundary conditions. It might be that the cases where it
would fail are already optimized to always-true or always-false, but
still...
Also, it doesn't look like this does the right thing for alt_op in the
csae of unsigned comparisons?
> +/* Checks if the FPU comparison in *CMP, *OP1, and *OP2 can be supported in
> + the current configuration. Perform modifications if MODIFY_P is true.
Double space. Multiple occurrences in this file.
> +/* Return true if register REGNO is a valid index register.
> + STRICT_P is true if REG_OK_STRICT is in effect. */
It's a bit strange to have definitions for index registers given that
MAX_REGS_PER_ADDRESS is defined to 1 for this port. One or the other
must be wrong.
> + case LABEL_REF:
> + case CONST_INT:
> + case CONST:
> + case CONST_DOUBLE:
> + /* ??? In here I need to add gp addressing. */
> + ret_val = false;
Address this comment.
> + return ret_val;
Could really just return directly from the switch cases without the
extra variable.
> + /* ??? these string names need moving into
> + an array in some header file */
Don't think so. Lose comment or fix it up to conform to coding style.
> +/* Implement TARGET_ENCODE_SECTION_INFO. */
This function together with
> +/* Set if this has a weak declaration. */
> +#define SYMBOL_FLAG_WEAK_DECL (1 << SYMBOL_FLAG_MACH_DEP_SHIFT)
> +#define SYMBOL_REF_WEAK_DECL_P(RTX) \
> + ((SYMBOL_REF_FLAGS (RTX) & SYMBOL_FLAG_WEAK_DECL) != 0)
seems a little odd, can the latter not simply be
SYMBOL_REF_DECL (RTX) ? DECL_WEAK (SYMBOL_REF_DECL (RTX)) : 0
rather than mucking about with symbol flags?
> +/* Print the operand OP to file stream
> + FILE modified by LETTER. LETTER
> + can be one of:
Strange line breaks.
> + if (CONSTANT_P (op) && (op != const0_rtx))
Lose extra parens.
> + fprintf (stderr, "Missing way to print (%c) ", letter);
> + debug_rtx (op);
> + gcc_unreachable ();
> +}
and
> + fprintf (stderr, "Missing way to print address\n");
> + debug_rtx (op);
> + gcc_unreachable ();
Should use output_operand_lossage since this can be caused by user error
in asms.
> +static const char*
Space. Multiple occurrences.
> +/* Instruction scheduler related. */
> +
> +static int
> +nios2_issue_rate (void)
> +{
> +#ifdef MAX_DFA_ISSUE_RATE
> + return MAX_DFA_ISSUE_RATE;
> +#else
> + return 1;
> +#endif
> +}
Hmm. If it's just 1 in reality then the default definition is sufficient
and we don't need this function.
> + if (mode == BLKmode)
> + {
> + param_size = int_size_in_bytes (type);
> + gcc_assert (param_size >= 0);
> + /*
> + if (param_size < 0)
> + internal_error
> + ("Do not know how to handle large structs or variable length
> types");
> + */
Deal with the problem if necessary then remove both copies of the
commented-out code.
> + return;
Lose this (in arg_advance).
> + /* ??? Do we need to treat floating point specially, ala MIPS? */
Do you?
> + return (regno == FIRST_RETVAL_REGNO);
Parens.
> +/* ??? It may be possible to eliminate the copyback and implement
> + my own va_arg type, but that is more work for now. */
Function could use a different comment.
> + /* This way of constructing the function tree types will slightly
> + overlap with the N2_FTYPES list used by other builtins. */
What does that mean?
> + if (!custom_insn_opcode (value, VOIDmode))
> + error ("Custom instruction opcode must be compile time "
> + "constant in the range 0-255 for __builtin_custom_%s",
> + custom_builtin_name[index]);
I think errors should start with lower case.
> + emit_insn (insn);
> + return has_target_p ? target : const0_rtx;
> +}
> +
> +
> +
> +
More excess newlines.
> +/* Implement TARGET_INIT_BUILTINS. */
It looks like in this area we're missing a definition of
TARGET_BUILTIN_DECL. AFAIR not defining that breaks LTO, do you see such
a problem on this target?
> +nios2_expand_builtin_insn (const struct nios2_builtin_desc *d, int n,
> + struct expand_operand* ops, bool has_target_p)
Again, space before "*", not after.
> +static rtx
> +nios2_expand_ldstio_builtin (tree exp, rtx target,
> + const struct nios2_builtin_desc *d)
Excess space here.
> + /* stxio */
> + /* ldxio */
Comment style.
> +/* Implement TARGET_INIT_LIBFUNCS. */
> +static void
> +nios2_init_libfuncs (void)
> +{
> + /* For Linux, we have access to kernel support for atomic operations. */
> + if (TARGET_LINUX_ABI)
> + init_sync_libfuncs (UNITS_PER_WORD);
> +}
> +
> +
> +
> +
Newlines.
> + /* Code conflicts between different __builtin_custom_xnxx calls
> + do not seem to be checked. ??? */
Is that a problem?
> +/* Remember the last target of nios2_set_current_function. */
> +static GTY(()) tree nios2_previous_fndecl;
Ok, I don't know any of this stuff. It looks sufficiently similar to
what i386 is doing that I guess it's ok.
> +
> +/* Basic Characteristics of Registers:
> +Register Number
Comment formatting.
> +#define SC_REGNO (12)
> +#define STATIC_CHAIN_REGNUM SC_REGNO
There are a lot of these pairs, and it looks like unnecessary double
indirection. Lose the former in all cases. No parens around constants.
> + D00_REG,
> + D01_REG,
> + D02_REG,
See previous comment about these classes.
> +#define GP_REGNO_FIRST 0
> +#define GP_REGNO_LAST RA_REGNO
> +#define GP_NREGS (GP_REGNO_LAST - GP_REGNO_FIRST + 1)
> +#define GP_REGNO_P(REGNO) ((unsigned) ((REGNO) - GP_REGNO_FIRST) < GP_NREGS)
Seems a little overly complex.
> +#define REGNO_REG_CLASS(REGNO) (GP_REGNO_P (REGNO) ? GP_REGS : ALL_REGS)
Well, if you do decide to keep the Dxx_REG classes... strictly speaking
they should be returned, although it shouldn't really matter and this
definition is probably better in practice.
> +#define INDEX_REG_CLASS ALL_REGS
See previous comment about MAX_REGS_PER_ADDRESS.
> +#define SMALL_INT(X) ((X) >= -0x8000 && (X) < 0x8000)
> +#define SMALL_INT_UNSIGNED(X) ((X) >= 0 && (X) < 0x10000)
> +#define UPPER16_INT(X) (((X) & 0xffff) == 0)
> +#define SHIFT_INT(X) ((X) >= 0 && (X) <= 31)
> +#define RDWRCTL_INT(X) ((X) >= 0 && (X) <= 31)
> +#define CUSTOM_INSN_OPCODE(X) ((X) >= 0 && (X) <= 255)
Minor, but there's not really a consistent naming convention apparent
for these.
> +/* Stack Layout and Calling Conventions. */
> +
> +/* The downward variants are used by the compiler,
> + the upward ones serve as documentation. */
> +#define STACK_GROWS_DOWNWARD
> +#define FRAME_GROWS_UPWARD
> +#define ARGS_GROW_UPWARD
Ugh, no. If you must, #define FRAME_GROWS_DOWNWARD 0, but do not define
bogus macros.
> +/* Treat LOC as a byte offset from the stack pointer and round it up
> + to the next fully-aligned offset. */
> +#define STACK_ALIGN(LOC) \
> + (((LOC) + ((PREFERRED_STACK_BOUNDARY / 8) - 1)) \
> + & ~((PREFERRED_STACK_BOUNDARY / 8) - 1))
Should move to nios2.c near where it is actually used.
> +/* Calling convention definitions. */
> +typedef struct nios2_args
> +{
> + int regs_used;
> +} CUMULATIVE_ARGS;
Minor, but could be typedef int CUMULATIVE_ARGS if you want.
> +/* This is to initialize the above unused CUM data type. */
> +#define INIT_CUMULATIVE_ARGS(CUM, FNTYPE, LIBNAME, FNDECL, N_NAMED_ARGS) \
> + (nios2_init_cumulative_args (&CUM, FNTYPE, LIBNAME, FNDECL, N_NAMED_ARGS))
Could be done in-line since it only sets a single value.
> +#define SLOW_BYTE_ACCESS 1
Is this intentional?
> +/* It is as good to call a constant function address as to call an address
> + kept in a register.
> + ??? Not true anymore really. Now that call cannot address full range
> + of memory callr may need to be used */
Hmm.
> +@item T
> +A @code{const} wrapped @code{UNSPEC} expression,
> +representing a supported PIC or TLS relocation.
Might want to place this inside an @ifset INTERNALS, since there
probably isn't a way for user code to make use of this.
> -The @code{target} attribute is not implemented in GCC versions earlier
> -than 4.4 for the i386/x86_64 and 4.6 for the PowerPC back ends. It is
> -not currently implemented for other back ends.
I suppose the paragraph should be extended for nios (as it is in the
pragma documentation) rather than lost.
> +or void if absent. The @code{n} represnts the first parameter
> +The letters reprsent the following data types:
"represent", twice.
Bernd
