Hello Michael,
Thanks for reviewing in such detail. Please see my answers below. I will
incorporate many of your suggestions immediately.
Thanks,
Pompa
About your general comments:
>1) Your formatting of the lisp code in the cr16.md is inconsistant. The
> simplest approach is to go in with gnu emacs, and hit tab at the
beginning
> of the line to let emacs reformat it. For example, instead of:
I will make necessary modification.
>3) Some places, particularly in the .md file you don't have the
space before
> the '(' in calls. For example instead of:
I will make necessary modification.
>4) You call abort () in several places. Those should be changed to
> gcc_unreachable ().
I guess, this is very important. I will do it right away.
>5) There are several places that when you split up a long line, you
have the
> operator at the end of the line instead of at the beginning of the
next
> line. For example instead of:
I will make necessary modification.
About your specific comments:
>1) I am somewhat wary of you mov<mode> code. You probably need to
define the
> appropriate reload_{out,in}<mode> operations to properly move stuff
between
> registers, and do the reloading.
Can you please explain what was the observation. This could be a
major change to the port.
Therefore I would want to recode it is carefully, if necessary.
>2) Technically fixing the size of double to 32-bits violates
ANSI/ISO C, but
> having supported 16-bit embedded platforms in the past, floating
point is
> often just not done in that environment. You might get several test
> failures that want to use 64-bit floating point. I see that you don't
> support trampolines either, you might document this restrictions in
the
> texinfo file on CR16 (and any other restrictions that you have).
Well, I can't help. CR16 has no support for 64-bit arithmetic and I
guess, it is
not intended as well. I will document the unsupported
features/restrictions as well.
>3) SIGNED_INT_FITS_N_BITS and UNSIGNED_INT_FITS_N_BITS uses 'long
long'. You
> should use HOST_WIDE_INT instead of long long.
Important point. I will do it right away.
>4) You defined REG_CLASS_FROM_LETTER. At some point in the future, you
> probably should migrate this to using define_register_constraint in
the .md
> file.
May be it is right time to migrate. I will plan for this.
>5) On the insv define_expand, you probably should define 2 new
predicates and
> eliminate the tests inside of the {}. One predicate would only match a
> const_int 1, and the other would match 0 or 1. So instead of:
Your suggestion is much better. I will incorporate and test it.
>6) cr16_prepare_push_pop_string allocates memory which is never
freed up.
> Either use a static buffer, or use asm_printf and print out the string
> directly.
Yes. This is an issue. I will incorporate your suggestion.
>7) The HI division is generic, this might be useful to move into
libgcc2 for
> other 16-bit ports (I dunno whether there are more 16 bit ports).
I did not want to change the files that would affect other ports.
May be in future I will submit another patch.
>8) In ashr<mode> and lshr<mode>, you use copy_to_mode_reg, when
instead you
> should use predicates to restrict the type to the appropriate types:
I will incorporate your suggestion.