This is the mail archive of the
`gcc-patches@gcc.gnu.org`
mailing list for the GCC project.

Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|

Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |

Other format: | [Raw text] |

*From*: Richard Biener <richard dot guenther at gmail dot com>*To*: Kenneth Zadeck <zadeck at naturalbridge dot com>*Cc*: Mike Stump <mikestump at comcast dot net>, gcc-patches <gcc-patches at gcc dot gnu dot org>, Lawrence Crowl <crowl at google dot com>, rdsandiford at googlemail dot com, Ian Lance Taylor <iant at google dot com>*Date*: Wed, 24 Apr 2013 14:09:37 +0200*Subject*: Re: patch to fix constant math -5th patch, rtl*References*: <506C72C7 dot 7090207 at naturalbridge dot com> <CAFiYyc1=8=LffiZ=EDBOy_uzn_ARdXk1OWxT=abYd8ot+iFp5Q at mail dot gmail dot com> <50891AAC dot 8090301 at naturalbridge dot com> <CAFiYyc15kmhRWhN3tsZqJDbZ5Uh6tVa45ssiYdsytLEfqaZ4zw at mail dot gmail dot com> <87y5im3orb dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <CAFiYyc2buJtu8RMKnLnvvb-A2=aYwopO+RBLPO6iJ3gKnq-hvg at mail dot gmail dot com> <87pq3y3kyk dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <CAFiYyc3NjOxpQ-Y9GDrQOET+dc3LXWuiuM=DxqmyASE8urRoWw at mail dot gmail dot com> <50912D85 dot 1070002 at naturalbridge dot com> <CAFiYyc2Q2UQPmkhExi2c8f-BSGLv+Rq1rOy4NdPQmTqSRE1A0A at mail dot gmail dot com> <5091331C dot 3030504 at naturalbridge dot com> <CAFiYyc1L6zuehE75dEfd_fB1-81F1fDHpL3kS=tbk6qAK3Texg at mail dot gmail dot com> <512D686B dot 90000 at naturalbridge dot com> <CAFiYyc3fXewAW2dU6-RHLiTQ-ZiLgdWmfwdFF6k1VqxPsrvZbQ at mail dot gmail dot com> <515EC4E7 dot 7040907 at naturalbridge dot com> <CAFiYyc3x6xb16kbYB9LzcC+XX17hM=R-h9KEWCZy6o0k-Csjfw at mail dot gmail dot com> <516DAF9B dot 3050008 at naturalbridge dot com> <516DB1F3 dot 8090908 at naturalbridge dot com>

On Tue, Apr 16, 2013 at 10:17 PM, Kenneth Zadeck <zadeck@naturalbridge.com> wrote: > Here is a refreshed version of the rtl changes for wide-int. the only > change from the previous versions is that the wide-int binary operations > have been simplified to use the new wide-int binary templates. Looking for from_rtx calls (to see where we get the mode/precision from) I see for example - o = rtx_to_double_int (outer); - i = rtx_to_double_int (inner); - - m = double_int::mask (width); - i &= m; - m = m.llshift (offset, HOST_BITS_PER_DOUBLE_INT); - i = i.llshift (offset, HOST_BITS_PER_DOUBLE_INT); - o = o.and_not (m) | i; - + + o = (wide_int::from_rtx (outer, GET_MODE (SET_DEST (temp))) + .insert (wide_int::from_rtx (inner, GET_MODE (dest)), + offset, width)); where I'd rather have the original code preserved as much as possible and not introduce a new primitive wide_int::insert for this. The conversion and review process will be much more error-prone if we do multiple things at once (and it might keep the wide_int initial interface leaner). Btw, the wide_int::insert implementation doesn't assert anything about the inputs precision. Instead it reads + if (start + width >= precision) + width = precision - start; + + mask = shifted_mask (start, width, false, precision); + tmp = op0.lshift (start, 0, precision, NONE); + result = tmp & mask; + + tmp = and_not (mask); + result = result | tmp; which eventually ends up performing everything in target precision. So we don't really care about the mode or precision of inner. Then I see diff --git a/gcc/dwarf2out.h b/gcc/dwarf2out.h index ad03a34..531a7c1 100644 @@ -180,6 +182,7 @@ typedef struct GTY(()) dw_val_struct { HOST_WIDE_INT GTY ((default)) val_int; unsigned HOST_WIDE_INT GTY ((tag ("dw_val_class_unsigned_const"))) val_unsigned; double_int GTY ((tag ("dw_val_class_const_double"))) val_double; + wide_int GTY ((tag ("dw_val_class_wide_int"))) val_wide; dw_vec_const GTY ((tag ("dw_val_class_vec"))) val_vec; struct dw_val_die_union { ick. That makes dw_val_struct really large ... (and thus dw_attr_struct). You need to make this a pointer to a wide_int at least. -/* Return a CONST_INT or CONST_DOUBLE corresponding to target reading +/* Return a constant integer corresponding to target reading GET_MODE_BITSIZE (MODE) bits from string constant STR. */ static rtx c_readstr (const char *str, enum machine_mode mode) { - HOST_WIDE_INT c[2]; + wide_int c; ... - return immed_double_const (c[0], c[1], mode); + + c = wide_int::from_array (tmp, len, mode); + return immed_wide_int_const (c, mode); } err - what's this good for? It doesn't look necessary as part of the initial wide-int conversion at least. (please audit your patches for such cases) @@ -4994,12 +4999,12 @@ expand_builtin_signbit (tree exp, rtx target) if (bitpos < GET_MODE_BITSIZE (rmode)) { - double_int mask = double_int_zero.set_bit (bitpos); + wide_int mask = wide_int::set_bit_in_zero (bitpos, rmode); if (GET_MODE_SIZE (imode) > GET_MODE_SIZE (rmode)) temp = gen_lowpart (rmode, temp); temp = expand_binop (rmode, and_optab, temp, - immed_double_int_const (mask, rmode), + immed_wide_int_const (mask, rmode), NULL_RTX, 1, OPTAB_LIB_WIDEN); } else Likewise. I suppose you remove immed_double_int_const but I see no reason to do that. It just makes your patch larger than necessary. [what was the reason again to have TARGET_SUPPORTS_WIDE_INT at all? It's supposed to be a no-op conversion, right?] @@ -95,38 +95,9 @@ plus_constant (enum machine_mode mode, rtx x, HOST_WIDE_INT c) switch (code) { - case CONST_INT: - if (GET_MODE_BITSIZE (mode) > HOST_BITS_PER_WIDE_INT) - { - double_int di_x = double_int::from_shwi (INTVAL (x)); - double_int di_c = double_int::from_shwi (c); - - bool overflow; - double_int v = di_x.add_with_sign (di_c, false, &overflow); - if (overflow) - gcc_unreachable (); - - return immed_double_int_const (v, VOIDmode); - } - - return GEN_INT (INTVAL (x) + c); - - case CONST_DOUBLE: - { - double_int di_x = double_int::from_pair (CONST_DOUBLE_HIGH (x), - CONST_DOUBLE_LOW (x)); - double_int di_c = double_int::from_shwi (c); - - bool overflow; - double_int v = di_x.add_with_sign (di_c, false, &overflow); - if (overflow) - /* Sorry, we have no way to represent overflows this wide. - To fix, add constant support wider than CONST_DOUBLE. */ - gcc_assert (GET_MODE_BITSIZE (mode) <= HOST_BITS_PER_DOUBLE_INT); - - return immed_double_int_const (v, VOIDmode); - } - + CASE_CONST_SCALAR_INT: + return immed_wide_int_const (wide_int::from_rtx (x, mode) + + wide_int::from_shwi (c, mode), mode); you said you didn't want to convert CONST_INT to wide-int. But the above is certainly a lot less efficient than before - given your change to support operator+ RTX even less efficient than possible. The above also shows three 'mode' arguments while one to immed_wide_int_const would be enough (given it would truncate the arbitrary precision result from the addition to modes precision). That is, I see no reason to remove the CONST_INT case or the CONST_DOUBLE case. [why is the above not in any way guarded with TARGET_SUPPORTS_WIDE_INT?] What happens with overflows in the wide-int case? The double-int case asserted that there is no overflow across 2 * hwi precision, the wide-int case does not. Still the wide-int case now truncates to 'mode' precision while the CONST_DOUBLE case did not. That's a change in behavior, no? Effectively the code for CONST_INT and CONST_DOUBLE did "arbitrary" precision arithmetic (up to the precision they can encode) which wide-int changes. Can we in such cases please to a preparatory patch and change the CONST_INT/CONST_DOUBLE paths to do an explicit [sz]ext to mode precision first? What does wide-int do with VOIDmode mode inputs? It seems to ICE on them for from_rtx and use garbage (0) for from_shwi. Ugh. Btw, plus_constant asserts that mode is either VOIDmode (I suppose semantically do "arbitrary precision") or the same mode as the mode of x (I suppose semantically do "mode precision"). Neither the current nor your implementation seems to do something consistent here :/ So please, for those cases (I suppose there are many more, eventually one of the reasons why you think that requiring a mode for all CONST_DOUBLEs is impossible), can we massage the current code to 1) document what is desired, 2) follow that specification with regarding to computation mode / precision and result mode / precision? Thanks, Richard. > kenny >

**Follow-Ups**:**Re: patch to fix constant math -5th patch, rtl***From:*Richard Sandiford

**References**:**Re: patch to fix constant math - 4th patch - the wide-int class - patch ping for the next stage 1***From:*Richard Biener

**Re: patch to fix constant math - 4th patch - the wide-int class - patch ping for the next stage 1***From:*Kenneth Zadeck

**Re: patch to fix constant math - 4th patch - the wide-int class - patch ping for the next stage 1***From:*Richard Biener

**Re: patch to fix constant math - 4th patch - the wide-int class - patch ping for the next stage 1***From:*Kenneth Zadeck

**Re: patch to fix constant math -5th patch, rtl***From:*Kenneth Zadeck

Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|

Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |