> On 09/19/14 22:04, Jan Hubicka wrote:
> >Hi,
> >int_bit_position is used by ipa-devirt's type walking code. It is currently
> >a bottleneck
> >since I introduced speculation into contextes (I plan to solve this by
> >changing the
> >way i cache results). But this patch seems to make sense anyway: we do not
> >need to go
> >through folding:
> >tree
> >bit_from_pos (tree offset, tree bitpos)
> >{
> > if (TREE_CODE (offset) == PLUS_EXPR)
> > offset = size_binop (PLUS_EXPR,
> > fold_convert (bitsizetype, TREE_OPERAND (offset,
> > 0)),
> > fold_convert (bitsizetype, TREE_OPERAND (offset,
> > 1)));
> > else
> > offset = fold_convert (bitsizetype, offset);
> > return size_binop (PLUS_EXPR, bitpos,
> > size_binop (MULT_EXPR, offset, bitsize_unit_node));
> >}
> >
> >Because all the code cares only about constant offsets, we do not need to go
> >through fold_convert,
> >because all the codes go via int_bit_position that already expects result to
> >be host wide int,
> >it seems to make sense to implement quick path for that.
> >
> >Bootstrap/regtest x86_64 in progress, OK?
> >
> >Honza
> >
> > * stor-layout.c (int_bit_from_pos): New function.
> > * stor-layout.h (int_bit_from_pos): Declare.
> > * tree.c (int_bit_from_pos): Use it.
> Just as a note to anyone else that peeks at this code, tree_to_shwi
> will verify the nodes are constant during a checking build.
>
> I'd consider a different name for the function that somehow
> indicates the inputs are constants.
>
> jeff
>
> Please consider an assert or other checking code to ensure that
> OFFSET and BITPOS are constants. Oh, I see that tree_to_shwi will
> get that checking when it
>
> Jeff
Yep, tree_to_shwi will check it. The old code did generic expression folding
and
called tree_to_shwi on result, so the only difference is that old code will
accept
unfolded expressions that miraculously folds into constant. I think it is bug
to
have those in IL especially on places we do not expect variable offsets.
Based on that observation, I think we can also drop handling of PLUS_EXPR in
this case
as follows.
Concerning the function, it has documented in toplevel comment that parameter
must
be constant or it crashes, so I think it is fine. Conerning name, I am open for
renaming,
but we have those int_* variants in quite few copies, so I can do that
incrementally
(see int_byte_position and related functions in stor layout).
I am testing the following simplified (and inline) variant.
Perhaps we could do similar stuff for other int_* accessors even if they do not
sit on hot paths in my test, just for the sake of code size.
Honza
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,20 @@ 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);
+/* 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 tree_to_shwi (DECL_FIELD_OFFSET (field)) * BITS_PER_UNIT
+ + tree_to_shwi (DECL_FIELD_BIT_OFFSET (field));
+}
+
+
#define sizetype sizetype_tab[(int) stk_sizetype]
#define bitsizetype sizetype_tab[(int) stk_bitsizetype]
#define ssizetype sizetype_tab[(int) stk_ssizetype]
