This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Optimize manual byte swap implementations v2
Hi Richard,
size is the number of significant bytes in n? In this case it should
be a plain int. As the above is a host dependence but relevant
for target code generation IMHO n should be a integer type of
the same width on all hosts (thus sth like int64_t).
HOST_WIDEST_INT is 64 bit on almost every target I think.
+/* In order to limit the number of instructions considered while
+ looking for a bswap implementation this number defines the maximum
+ depth of the search. */
+static const int BSWAP_RECURSION_LIMIT = 8;
How did you come up with this number? Does it make sense to
make it a param?
This is smallest number which makes the testcase run successful. I'm
not sure about making this a param. Essentially if someone comes up with
a bswap implementation which is not recognized due to this value we
should simply increase it.
It might be worthwhile to have separate values for 32 and 64 bit?!
whenever you use int_cst_value you have to check first if the
INTEGER_CST fits in a HOST_WIDE_INT. So, replace all
of them with host_integerp (..., 1) and TREE_INT_CST_LOW.
Or, as the types will probably never be that large use
TREE_INT_CST_LOW unconditionally, which is a lot faster
here.
I intend to only look for byte swaps if the target type is 32 or 64 bits
in size and the only constants I'm looking at are operands of bit AND
operations. So I think it is safe to assume that the constant alway fits
into a HOST_WIDEST_INT - right?!
+ (HOST_WIDE_INT)n->size * BITS_PER_UNIT)
+ return false;
Note that you assume that BITS_PER_UNIT is 8 elsewhere, this may
not be true! So use 8 here as well.
Yes I am forcing BITS_PER_UNIT to be 8 by only executing the pass if
that's true nevertheless I would prefer BITS_PER_UNIT here instead of a
magic number.
watch out for conversions in the num_ops == 2 case! Which
kind of operations do you want to look at here? There are
more elaborate predicates available than checking the number of ops.
Ok. I'll replace these checks with
gimple_assign_rhs_class (stmt) == GIMPLE_UNARY_RHS
and
gimple_assign_rhs_class (stmt) == GIMPLE_BINARY_RHS
The new version of the patch is currently bootstrapping.
Bye,
-Andreas-