CRIS: Add support for __builtin_clz.

Hans-Peter Nilsson hans-peter.nilsson@axis.com
Sat Sep 1 12:14:00 GMT 2007


> Date: Sat, 1 Sep 2007 10:42:50 +0200
> From: Jesper Nilsson <jespern@axis.com>

> Unfortunately, using the __builtin_clz for older architectures that
> don't implement the lz instruction increases register usage, and
> adds additional jumps to subroutines, and so creates a slightly
> larger libgcc.a (by 80 bytes).
> Thus, the old handcrafted code is retained for this case.

I mistakenly believed that __builtin_clz would be open-coded,
but then it'd be something like this (shown where val is a
32-bit unsigned int):

 int res = val == 0 ? 32 : 0;
 if (val > 0xffff) { res += 16; val >>= 16;}
 if (val > 0xff) { res += 8; val >>= 8;}
 if (val > 0xf) { res += 4; val >>= 4;}
 if (val > 0x3) { res += 2; val >>= 2;}
 if (val > 0x1) { res += 1; val >>= 1;}
 res += val;

so the expansion would probably just be a little over the size
threshold anyway.  Maybe a small win over the table-driven code
in libgcc2.c (which does about the same thing but uses a table
for the last 8 bits); I'm not sure.

> Testing has been done for cris-axis-elf on i686-pc-linux-gnu.
> No new regressions found.

You should've mentioned the combination of -march= options you
tested and the fallout you mentioned from *other* test-cases
that didn't expect separate -march= options.

> 2007-09-01  Jesper Nilsson  <jesper.nilsson@axis.com>
> 
> 	* config/cris/arit.c: Change from using LZ assembler macro to
> 	__builtin_clz.
> 	* config/cris/cris.h (TARGET_HAS_LZ, CLZ_DEFINED_VALUE_AT_ZERO):
> 	Defined to describe availability and behavior of CLZ.
> 	* config/cris/cris.md (clzsi2): Implement using lz instruction.
> 
> testsuite/ChangeLog:
> 
> 2007-09-01  Jesper Nilsson  <jesper.nilsson@axis.com>
> 
> 	* gcc.target/cris/builtin_clz_v0.c: New testcase.
> 	* gcc.target/cris/builtin_clz_v3.c: New testcase.

Thanks.  A few nits:

> Index: config/cris/arit.c
> ===================================================================
> --- config/cris/arit.c	(revision 127959)
> +++ config/cris/arit.c	(working copy)
> @@ -48,8 +48,7 @@ Boston, MA 02110-1301, USA.
>  #include "config.h"
>  
>  #if defined (__CRIS_arch_version) && __CRIS_arch_version >= 3
> -#define LZ(v) __extension__ \
> - ({ int tmp_; __asm__ ("lz %1,%0" : "=r" (tmp_) : "r" (v)); tmp_; })
> +#define LZ 1
>  #endif

Nah, the LZ now looks a bit awkward.  Better:

>  #if defined (__CRIS_arch_version) && __CRIS_arch_version >= 3
> -#define LZ(v) __extension__ \
> - ({ int tmp_; __asm__ ("lz %1,%0" : "=r" (tmp_) : "r" (v)); tmp_; })
> +#define LZ __builtin_clz (v)
>  #endif

so the other changes in that file will not be needed.  The
ChangeLog entry would then be:

 	* config/cris/arit.c (LZ): When defined, define as __builtin_clz.

> Index: config/cris/cris.h
> ===================================================================
> --- config/cris/cris.h	(revision 127959)
> +++ config/cris/cris.h	(working copy)
> @@ -278,6 +278,7 @@ extern int target_flags;
>  #define CRIS_DEFAULT_CPU_VERSION CRIS_CPU_BASE
>  
>  #define TARGET_HAS_MUL_INSNS (cris_cpu_version >= CRIS_CPU_NG)
> +#define TARGET_HAS_LZ (cris_cpu_version >= CRIS_CPU_ETRAX4)
>  
>  #define CRIS_SUBTARGET_HANDLE_OPTION(x, y, z)
>  
> @@ -1126,6 +1127,7 @@ struct cum_args {int regs;};
>     (8 instruction sequences) or less.  */
>  #define MOVE_RATIO 9
>  
> +#define CLZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE)	((VALUE) = 32, 1)

There's a layout in cris.h corresponding to the documentation
nodes.  This is under the comment "Node: Costs".  I'll move it
under "Node: Misc" and drop the TAB.

>  /* Node: Sections */
>  
> Index: config/cris/cris.md

> +

Nah, no ^L after just one insn.  I'll remove the one before
"uminsi3" too.  They all match the heading "Various strange
insns that gcc likes". :)

> Content-Disposition: attachment; filename="builtin_clz_v0.c"
> 
> /* Check that we don't use the lz insn for clz by checking */
> /* assembler output.  lz insn was implemented in v3 (ETRAX4).  */
> /* { dg-do compile } */
> /* { dg-skip-if "" { "cris-*-elf" } { "-march*" } { "" } } */

You got it to work eventually?  Good!

There's also the trivial change to cris.opt I mentioned briefly
in RL once:

Index: cris.opt
===================================================================
--- cris.opt	(revision 128012)
+++ cris.opt	(working copy)
@@ -45,7 +45,6 @@ Target Report Mask(MUL_BUG)
 Work around bug in multiplication instruction
 
 ; TARGET_ETRAX4_ADD: Instruction-set additions from Etrax 4 and up.
-; (Just "lz", which we don't really generate from GCC -- yet).
 metrax4
 Target Report Mask(ETRAX4_ADD)
 Compile for ETRAX 4 (CRIS v3)

I'll just run an extra round of regtesting with
RUNTESTFLAGS=--target_board=cris-sim\{,-march=v3,-march=v8,-march=v10\}
and then hopefully check it in with those changes.

Thanks!

brgds, H-P



More information about the Gcc-patches mailing list