This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: CRIS: Add hardware support for __builtin_ctz
- From: Hans-Peter Nilsson <hans-peter dot nilsson at axis dot com>
- To: jesper dot nilsson at axis dot com
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Thu, 6 Sep 2007 02:30:27 +0200
- Subject: Re: CRIS: Add hardware support for __builtin_ctz
> Date: Tue, 4 Sep 2007 16:30:48 +0200
> From: Jesper Nilsson <jespern@axis.com>
> * longlong.h [__CRIS_arch_version >= 8] (count_trailing_zeros):
> Defined.
> * config/cris/cris.md (ctzsi2, cris_swap_bits): Implemented.
> * config/cris/cris.h (CTZ_DEFINED_VALUE_AT_ZERO): Defined.
Thanks. I fixed up spacing and committed (tabs not spaces,
feigning consistency; no ")" on a line by itself in cris.md;
spurious extra space in ChangeLog entry above).
IMHO, I'd write it as:
* longlong.h [__CRIS_arch_version >= 8] (count_trailing_zeros):
Define.
* config/cris/cris.md ("ctzsi2"): Implement.
("cris_swap_bits"): New pattern.
* config/cris/cris.h (CTZ_DEFINED_VALUE_AT_ZERO): Define.
...because the canonical form for ChangeLog entries is as a log
of changes in imperative form (to do on the unchanged version),
not in past tense. That's as per the GNU standard. The "new
pattern" instead of "implement" is because (splitting hairs) it
*is*; it's not an implementation of a standard pattern name.
The quotes are just my old habits (and add-change-log-entry in
Emacs-Lisp mode IIRC). But editing the entry you wrote besides
plain format corrections, would also be restricting an (albeit
limited) amount of author's freedom of expression, so I won't;
this time instead torturing you with an old-school nitpicking
rant. :)
>
> testsuite/ChangeLog:
>
> 2007-09-03 Jesper Nilsson <jesper.nilsson@axis.com>
>
> * gcc.target/cris/builtin_ctz_v3.c: New testcase.
> * gcc.target/cris/builtin_ctz_v8.c: New testcase.
For future reference, it'd simplify me applying them, if you
please send them not as pure code in a separate attachment, but
instead "svn add" new test-cases and let the "svn diff" include
them.
Another request:
> Index: config/cris/cris.h
> ===================================================================
> --- config/cris/cris.h (revision 128067)
> +++ config/cris/cris.h (working copy)
> @@ -1410,6 +1410,7 @@
> #define TRULY_NOOP_TRUNCATION(OUTPREC, INPREC) 1
>
> #define CLZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE) ((VALUE) = 32, 1)
> +#define CTZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE) ((VALUE) = 32, 1)
These should really both be "2"; see optabs.c:expand_ffs and
tm.texi. Please test that change.
IMHO, a 1 for CLZ_DEFINED_VALUE_AT_ZERO should mean "all about
CLZ/CTZ defined at zero", not "all but the optab entry" (when
the define_expand-generated RTL is undefined for 0) as the "all
but the optab" is the odd case. It seems most port wrongly uses 1.
(I may repeat that statement until I'm coerced into swapping the
values myself. :)
brgds, H-P