On Wed, Feb 27, 2013 at 2:59 AM, Kenneth Zadeck
<zad...@naturalbridge.com> wrote:
> This patch contains a large number of the changes requested by Richi. It
> does not contain any of the changes that he requested to abstract the
> storage layer. That suggestion appears to be quite unworkable.
>
> I believe that the wide-int class addresses the needs of gcc for performing
> math on any size integer irregardless of the platform that hosts the
> compiler. The interface is admittedly large, but it is large for a reason:
> these are the operations that are commonly performed by the client
> optimizations in the compiler.
>
> I would like to get this patch preapproved for the next stage 1.
Please clean from dead code like
+// using wide_int::;
and
+#ifdef DEBUG_WIDE_INT
+ if (dump_file)
+ debug_wh ("wide_int::from_shwi %s " HOST_WIDE_INT_PRINT_HEX ")\n",
+ result, op0);
+#endif
and
+#ifdef NEW_REP_FOR_INT_CST
+ /* This is the code once the tree level is converted. */
+ wide_int result;
+ int i;
+
+ tree type = TREE_TYPE (tcst);
+
+ result.bitsize = GET_MODE_BITSIZE (TYPE_MODE (type));
+ result.precision = TYPE_PRECISION (type);
+ result.len = TREE_INT_CST_LEN (tcst);
+ for (i = 0; i < result.len; i++)
+ result.val[i] = TREE_INT_CST_ELT (tcst, i);
+
+ return result;
+#else
this also shows the main reason I was asking for storage abstraction.
The initialization from tree is way too expensive.
+/* Convert a integer cst into a wide int expanded to BITSIZE and
+ PRECISION. This call is used by tree passes like vrp that expect
+ that the math is done in an infinite precision style. BITSIZE and
+ PRECISION are generally determined to be twice the largest type
+ seen in the function. */
+
+wide_int
+wide_int::from_tree_as_infinite_precision (const_tree tcst,
+ unsigned int bitsize,
+ unsigned int precision)
+{
I know you have converted everything, but to make this patch reviewable
I'd like you to strip the initial wide_int down to a bare minimum.
Only then people will have a reasonable chance to play with interface
changes (such as providing a storage abstraction).
+/* Check the upper HOST_WIDE_INTs of src to see if the length can be
+ shortened. An upper HOST_WIDE_INT is unnecessary if it is all ones
+ or zeros and the top bit of the next lower word matches.
+
+ This function may change the representation of THIS, but does not
+ change the value that THIS represents. It does not sign extend in
+ the case that the size of the mode is less than
+ HOST_BITS_PER_WIDE_INT. */
+
+void
+wide_int::canonize ()
this shouldn't be necessary - it's an optimization - and due to value
semantics (yes - I know you have a weird mix of value semantics
and modify-in-place in wide_int) the new length should be computed
transparently when creating a new value.
Well. Leaving wide-int.c for now.
+class wide_int {
+ /* Internal representation. */
+
+ /* VAL is set to a size that is capable of computing a full
+ multiplication on the largest mode that is represented on the
+ target. The full multiplication is use by tree-vrp. tree-vpn
+ currently does a 2x largest mode by 2x largest mode yielding a 4x
+ largest mode result. If operations are added that require larger
+ buffers, then VAL needs to be changed. */
+ HOST_WIDE_INT val[4 * MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT];
as you conver partial int modes in MAX_BITSIZE_MODE_ANY_INT the
above may come too short. Please properly round up.
+ unsigned short len;
+ unsigned int bitsize;
+ unsigned int precision;
I see we didn't get away with this mix of bitsize and precision. I'm probably
going to try revisit the past discussions - but can you point me to a single
place in the RTL conversion where they make a difference? Bits beyond
precision are either undefined or properly zero-/sign-extended. Implicit
extension beyond len val members should then provide in "valid" bits
up to bitsize (if anyone cares). That's how double-ints work on tree
INTGER_CSTs
which only care for precision, even with partial integer mode types
(ok, I never came along one of these beasts - testcase / target?).
[abstraction possibility - have both wide_ints with actual mode and
wide_ints with arbitrary bitsize/precision]
+ enum ShiftOp {
+ NONE,
+ /* There are two uses for the wide-int shifting functions. The
+ first use is as an emulation of the target hardware. The
+ second use is as service routines for other optimizations. The
+ first case needs to be identified by passing TRUNC as the value
+ of ShiftOp so that shift amount is properly handled according to the
+ SHIFT_COUNT_TRUNCATED flag. For the second case, the shift
+ amount is always truncated by the bytesize of the mode of
+ THIS. */
+ TRUNC
+ };
I think I have expressed my opinion on this. (and SHIFT_COUNT_TRUNCATED
should vanish - certainly wide-int shouldn't care, so doesn't double-int)
+ enum SignOp {
+ /* Many of the math functions produce different results depending
+ on if they are SIGNED or UNSIGNED. In general, there are two
+ different functions, whose names are prefixed with an 'S" and
+ or an 'U'. However, for some math functions there is also a
+ routine that does not have the prefix and takes an SignOp
+ parameter of SIGNED or UNSIGNED. */
+ SIGNED,
+ UNSIGNED
+ };
See above. GCC standard practice is a 'unsigned uns' parameter.
+ inline static wide_int from_hwi (HOST_WIDE_INT op0, const_tree type);
+ inline static wide_int from_hwi (HOST_WIDE_INT op0, const_tree type,
+ bool *overflow);
+ inline static wide_int from_shwi (HOST_WIDE_INT op0, enum machine_mode mode);
+ inline static wide_int from_shwi (HOST_WIDE_INT op0, enum machine_mode mode,
+ bool *overflow);
+ inline static wide_int from_uhwi (unsigned HOST_WIDE_INT op0,
+ enum machine_mode mode);
+ inline static wide_int from_uhwi (unsigned HOST_WIDE_INT op0,
+ enum machine_mode mode,
+ bool *overflow);
way too many overloads. Besides the "raw" ones I only expect wide-ints
to be constructed from a tree INTEGER_CST or a rtx DOUBLE_INT.
+ inline static wide_int minus_one (unsigned int bitsize, unsigned int prec);
+ inline static wide_int minus_one (const_tree type);
+ inline static wide_int minus_one (enum machine_mode mode);
+ inline static wide_int minus_one (const wide_int &op1);
wide-ints representation should be properly sign-extended, thus all of the
possible -1 variants have the same representation. If it were not for
providing precision and bitsize for which you have the four overloads.
Just have one. Providing precision (or bitsize).
I see that for example in wide_int::add bitsize and precision are arbitrarily
copied from the first operand. With the present way of dealing with them
it would sound more logical to assert that they are the same for both
operands (do both need to match?). I'd rather see wide-int being
"arbitrary" precision/bitsize up to its supported storage size (as
double-int is).
I suppose we've been there and discussed this to death already though ;)
As you have some fused operation plus sign-/zero-extend ops already
the alternative is to always provide a precision for the result and treat the
operands as "arbitrary" precision (that way wide_int::minus_one can
simply return a sign-extended precision 1 -1).
Btw, wide_int::add does not look at bitsize at all, so it clearly is redundant
information. Grepping for uses of bitsize shows up only maintaining and
copying around this information as well. please remove bitsize.
Ok, enough for today.
Thanks,
Richard.
> kenny
>
