Speedup int_bit_from_pos

Hi,
it now seems to work, finaly :)
Bootstrapped/regtested x86_64-linux, OK?

	* tree.c (int_bit_position): Move to...
	* tree.h (int_bit_position): ... here; implement using
	direct wide_int calculation instead of folding.

On Mon, 22 Sep 2014, Jan Hubicka wrote:

> Hi,
> it now seems to work, finaly :)
> Bootstrapped/regtested x86_64-linux, OK?
> 
> 	* tree.c (int_bit_position): Move to...
> 	* tree.h (int_bit_position): ... here; implement using
> 	direct wide_int calculation instead of folding.
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 215421)
> +++ tree.c	(working copy)
> @@ -2831,16 +2831,6 @@ bit_position (const_tree field)
>    return bit_from_pos (DECL_FIELD_OFFSET (field),
>  		       DECL_FIELD_BIT_OFFSET (field));
>  }
> -
> -/* Likewise, but return as an integer.  It must be representable in
> -   that way (since it could be a signed value, we don't have the
> -   option of returning -1 like int_size_in_byte can.  */
> -
> -HOST_WIDE_INT
> -int_bit_position (const_tree field)
> -{
> -  return tree_to_shwi (bit_position (field));
> -}
>  
>  /* Return the byte position of FIELD, in bytes from the start of the record.
>     This is a tree of type sizetype.  */
> Index: tree.h
> ===================================================================
> --- tree.h	(revision 215421)
> +++ tree.h	(working copy)
> @@ -3877,10 +3877,9 @@ extern tree size_in_bytes (const_tree);
>  extern HOST_WIDE_INT int_size_in_bytes (const_tree);
>  extern HOST_WIDE_INT max_int_size_in_bytes (const_tree);
>  extern tree bit_position (const_tree);
> -extern HOST_WIDE_INT int_bit_position (const_tree);
>  extern tree byte_position (const_tree);
>  extern HOST_WIDE_INT int_byte_position (const_tree);
>  
>  #define sizetype sizetype_tab[(int) stk_sizetype]
>  #define bitsizetype sizetype_tab[(int) stk_bitsizetype]
>  #define ssizetype sizetype_tab[(int) stk_ssizetype]
> @@ -4797,4 +4797,14 @@ extern tree get_inner_reference (tree, H
>     EXP, an ARRAY_REF or an ARRAY_RANGE_REF.  */
>  extern tree array_ref_low_bound (tree);
>  
> +/* Like bit_position, but return as an integer.  It must be representable in
> +   that way (since it could be a signed value, we don't have the
> +   option of returning -1 like int_size_in_byte can.  */
> +
> +static inline HOST_WIDE_INT

^^^^

ok with static removed (and yes, we should change all our inline
functions that are not used by any C program that way - watch out
for generator programs).

Thanks,
Richard.

> +int_bit_position (const_tree field)
> +{ 
> +  return (wi::lrshift (wi::to_offset (DECL_FIELD_OFFSET (field)), BITS_PER_UNIT_LOG)
> +	  + wi::to_offset (DECL_FIELD_BIT_OFFSET (field))).to_shwi ();
> +}
>  #endif  /* GCC_TREE_H  */
> 
>

Jan Hubicka <hubicka@ucw.cz> writes:
> +/* Like bit_position, but return as an integer.  It must be representable in
> +   that way (since it could be a signed value, we don't have the
> +   option of returning -1 like int_size_in_byte can.  */
> +
> +static inline HOST_WIDE_INT
> +int_bit_position (const_tree field)
> +{ 
> +  return (wi::lrshift (wi::to_offset (DECL_FIELD_OFFSET (field)), BITS_PER_UNIT_LOG)
> +	  + wi::to_offset (DECL_FIELD_BIT_OFFSET (field))).to_shwi ();
> +}

Should this be lshift (left shift) rather than lrshift (logical right shift)?

Thanks for doing this BTW.

Richard

> Jan Hubicka <hubicka@ucw.cz> writes:
> > +/* Like bit_position, but return as an integer.  It must be representable in
> > +   that way (since it could be a signed value, we don't have the
> > +   option of returning -1 like int_size_in_byte can.  */
> > +
> > +static inline HOST_WIDE_INT
> > +int_bit_position (const_tree field)
> > +{ 
> > +  return (wi::lrshift (wi::to_offset (DECL_FIELD_OFFSET (field)), BITS_PER_UNIT_LOG)
> > +	  + wi::to_offset (DECL_FIELD_BIT_OFFSET (field))).to_shwi ();
> > +}
> 
> Should this be lshift (left shift) rather than lrshift (logical right shift)?

Yep, I noticed the bug and corrected it earlier.
I suppose it may be interesting to get coverage of GCC and looks for cases where we
invoke fold for doing constant arithmetic. int_bit_position is definitely not alone.

Honza
> 
> Thanks for doing this BTW.
> 
> Richard