| Message ID | 5293FE59.6090801@naturalbridge.com |
|---|---|
| State | New |
| Headers | show |
> you are correct - this was an incorrect change. I believe that the > patch below would be correct, but it is impossible to test it because (i > believe) that gcc no longer works if the host_bits_per_wide_int is 32. > I could be wrong about this but if i am correct, what do you want me to do? While you're right that most mainstream architectures now require a 64-bit HWI, not all of them do according to config.gcc, so I don't think that this path is entirely dead yet. I'll carry out the testing once we agree on the final change. The checks were written that way in UI_From_gnu to be efficient, so can't we find something more efficient than wi::gtu_p <wi::lrshift> ? We need to return No_Uint on Input not representing a 64-bit signed integer, can't this be done by testing the number of HWIs in Input and its sign bit if its type is unsigned?
On Tue, Nov 26, 2013 at 10:18 AM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> you are correct - this was an incorrect change. I believe that the >> patch below would be correct, but it is impossible to test it because (i >> believe) that gcc no longer works if the host_bits_per_wide_int is 32. >> I could be wrong about this but if i am correct, what do you want me to do? > > While you're right that most mainstream architectures now require a 64-bit > HWI, not all of them do according to config.gcc, so I don't think that this > path is entirely dead yet. I'll carry out the testing once we agree on the > final change. > > The checks were written that way in UI_From_gnu to be efficient, so can't we > find something more efficient than wi::gtu_p <wi::lrshift> ? We need to > return No_Uint on Input not representing a 64-bit signed integer, can't this > be done by testing the number of HWIs in Input and its sign bit if its type is > unsigned? I suppose a generic wi::fits_[su]i (wi, <number-of-bits>) could be implemented, similar to how we have int_fits_type_p (not sure if the current wide-int implementation of that is most effective, but it falls back to double-int-ish template <typename T> bool wi::fits_to_tree_p (const T &x, const_tree type) { if (TYPE_SIGN (type) == UNSIGNED) return eq_p (x, zext (x, TYPE_PRECISION (type))); else return eq_p (x, sext (x, TYPE_PRECISION (type))); } which looks more efficient than shifting. Richard. > -- > Eric Botcazou
On 26/11/13 09:18, Eric Botcazou wrote: >> you are correct - this was an incorrect change. I believe that the >> patch below would be correct, but it is impossible to test it because (i >> believe) that gcc no longer works if the host_bits_per_wide_int is 32. >> I could be wrong about this but if i am correct, what do you want me to do? > > While you're right that most mainstream architectures now require a 64-bit > HWI, not all of them do according to config.gcc, so I don't think that this > path is entirely dead yet. I'll carry out the testing once we agree on the > final change. I'm hoping, once this patch series is in that we might be able to migrate the ARM port back to supporting a 32-bit HWI. The driving factor behind the original switch was supporting 128-bit constants for Neon and these patches should resolve that. R.
On Tue, Nov 26, 2013 at 2:44 PM, Richard Earnshaw <rearnsha@arm.com> wrote: > On 26/11/13 09:18, Eric Botcazou wrote: >>> you are correct - this was an incorrect change. I believe that the >>> patch below would be correct, but it is impossible to test it because (i >>> believe) that gcc no longer works if the host_bits_per_wide_int is 32. >>> I could be wrong about this but if i am correct, what do you want me to do? >> >> While you're right that most mainstream architectures now require a 64-bit >> HWI, not all of them do according to config.gcc, so I don't think that this >> path is entirely dead yet. I'll carry out the testing once we agree on the >> final change. > > I'm hoping, once this patch series is in that we might be able to > migrate the ARM port back to supporting a 32-bit HWI. The driving > factor behind the original switch was supporting 128-bit constants for > Neon and these patches should resolve that. i?86 would be another candidate (if you don't build a compiler with -m64 support). Richard. > R. > >
On Tue, Nov 26, 2013 at 5:55 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Tue, Nov 26, 2013 at 2:44 PM, Richard Earnshaw <rearnsha@arm.com> wrote: >> On 26/11/13 09:18, Eric Botcazou wrote: >>>> you are correct - this was an incorrect change. I believe that the >>>> patch below would be correct, but it is impossible to test it because (i >>>> believe) that gcc no longer works if the host_bits_per_wide_int is 32. >>>> I could be wrong about this but if i am correct, what do you want me to do? >>> >>> While you're right that most mainstream architectures now require a 64-bit >>> HWI, not all of them do according to config.gcc, so I don't think that this >>> path is entirely dead yet. I'll carry out the testing once we agree on the >>> final change. >> >> I'm hoping, once this patch series is in that we might be able to >> migrate the ARM port back to supporting a 32-bit HWI. The driving >> factor behind the original switch was supporting 128-bit constants for >> Neon and these patches should resolve that. > > i?86 would be another candidate (if you don't build a compiler with -m64 > support). Not true for x86 since we have Variable HOST_WIDE_INT ix86_isa_flags = TARGET_64BIT_DEFAULT | TARGET_SUBTARGET_ISA_DEFAULT in i386.opt. We need more than 32 bits for ix86_isa_flags.
On 11/26/2013 08:44 AM, Richard Earnshaw wrote: > On 26/11/13 09:18, Eric Botcazou wrote: >>> you are correct - this was an incorrect change. I believe that the >>> patch below would be correct, but it is impossible to test it because (i >>> believe) that gcc no longer works if the host_bits_per_wide_int is 32. >>> I could be wrong about this but if i am correct, what do you want me to do? >> While you're right that most mainstream architectures now require a 64-bit >> HWI, not all of them do according to config.gcc, so I don't think that this >> path is entirely dead yet. I'll carry out the testing once we agree on the >> final change. > I'm hoping, once this patch series is in that we might be able to > migrate the ARM port back to supporting a 32-bit HWI. The driving > factor behind the original switch was supporting 128-bit constants for > Neon and these patches should resolve that. > > R. Richard, I had not realized that you were into self abuse like that. you are going to have a bad time. I tried this as a way to test the wide-int branch because if we made hwi be 32bits, then it would trigger the long version of the implementation wide-int routines. What a disaster!!!! richard sandiford told me not to try and i ignored him. that was a wasted weekend!!! There are roadblocks everywhere. i certainly do not have a complete list, because i gave up after hacking a few things back. But very quickly you find places like real.[ch] which no longer work because the math that the dfp people hacked into it no longer works if the hwis are 32 bits. it is misleading because all of the tests are still there, it is just that the equations return the wrong answers. It is like this, one place after another. As far as your hope that once this patch goes in you would be able to support 128 bit constants is only partially true. it will/should be the case, that you always get the correct answer and the code will not ice. That is the good news. But the bad news is that there is still way to much code of the form, if var fits in hwi, do some optimization, else do not do the optimization. Richi did not want me to attack this code yet because it would have made the patches even bigger. For the rtl level a lot of this was removed because i did it before he told me not to, but the tree level still has tons of code like this. So at least for a long time, like the close of the next release, this is not going to get fixed. Kenny >
> On Nov 26, 2013, at 6:00 AM, "H.J. Lu" <hjl.tools@gmail.com> wrote: > > On Tue, Nov 26, 2013 at 5:55 AM, Richard Biener > <richard.guenther@gmail.com> wrote: >> On Tue, Nov 26, 2013 at 2:44 PM, Richard Earnshaw <rearnsha@arm.com> wrote: >>> On 26/11/13 09:18, Eric Botcazou wrote: >>>>> you are correct - this was an incorrect change. I believe that the >>>>> patch below would be correct, but it is impossible to test it because (i >>>>> believe) that gcc no longer works if the host_bits_per_wide_int is 32. >>>>> I could be wrong about this but if i am correct, what do you want me to do? >>>> >>>> While you're right that most mainstream architectures now require a 64-bit >>>> HWI, not all of them do according to config.gcc, so I don't think that this >>>> path is entirely dead yet. I'll carry out the testing once we agree on the >>>> final change. >>> >>> I'm hoping, once this patch series is in that we might be able to >>> migrate the ARM port back to supporting a 32-bit HWI. The driving >>> factor behind the original switch was supporting 128-bit constants for >>> Neon and these patches should resolve that. >> >> i?86 would be another candidate (if you don't build a compiler with -m64 >> support). > > Not true for x86 since we have > > Variable > HOST_WIDE_INT ix86_isa_flags = TARGET_64BIT_DEFAULT | > TARGET_SUBTARGET_ISA_DEFAULT > > in i386.opt. We need more than 32 bits for ix86_isa_flags. Then that should be HOST_WIDEST_INT instead. Thanks, Andrew > > -- > H.J.
On Tue, Nov 26, 2013 at 3:00 PM, Kenneth Zadeck <zadeck@naturalbridge.com> wrote: > > On 11/26/2013 08:44 AM, Richard Earnshaw wrote: >> >> On 26/11/13 09:18, Eric Botcazou wrote: >>>> >>>> you are correct - this was an incorrect change. I believe that the >>>> patch below would be correct, but it is impossible to test it because (i >>>> believe) that gcc no longer works if the host_bits_per_wide_int is 32. >>>> I could be wrong about this but if i am correct, what do you want me to >>>> do? >>> >>> While you're right that most mainstream architectures now require a >>> 64-bit >>> HWI, not all of them do according to config.gcc, so I don't think that >>> this >>> path is entirely dead yet. I'll carry out the testing once we agree on >>> the >>> final change. >> >> I'm hoping, once this patch series is in that we might be able to >> migrate the ARM port back to supporting a 32-bit HWI. The driving >> factor behind the original switch was supporting 128-bit constants for >> Neon and these patches should resolve that. >> >> R. > > > Richard, > > I had not realized that you were into self abuse like that. you are > going to have a bad time. I tried this as a way to test the wide-int branch > because if we made hwi be 32bits, then it would trigger the long version of > the implementation wide-int routines. What a disaster!!!! richard sandiford > told me not to try and i ignored him. that was a wasted weekend!!! There > are roadblocks everywhere. i certainly do not have a complete list, > because i gave up after hacking a few things back. But very quickly you > find places like real.[ch] which no longer work because the math that the > dfp people hacked into it no longer works if the hwis are 32 bits. it is > misleading because all of the tests are still there, it is just that the > equations return the wrong answers. It is like this, one place after > another. > > As far as your hope that once this patch goes in you would be able to > support 128 bit constants is only partially true. it will/should be the > case, that you always get the correct answer and the code will not ice. > That is the good news. But the bad news is that there is still way to > much code of the form, if var fits in hwi, do some optimization, else do not > do the optimization. Richi did not want me to attack this code yet because > it would have made the patches even bigger. For the rtl level a lot of > this was removed because i did it before he told me not to, but the tree > level still has tons of code like this. So at least for a long time, like > the close of the next release, this is not going to get fixed. Note that in the long run we want wide-int to use HOST_WIDEST_FAST_INT and not HOST_WIDE_INT internally. I know I asked you to do that but it's of course too late now. Richard. > Kenny > > > >> >
On Tue, Nov 26, 2013 at 6:03 AM, <pinskia@gmail.com> wrote: > > >> On Nov 26, 2013, at 6:00 AM, "H.J. Lu" <hjl.tools@gmail.com> wrote: >> >> On Tue, Nov 26, 2013 at 5:55 AM, Richard Biener >> <richard.guenther@gmail.com> wrote: >>> On Tue, Nov 26, 2013 at 2:44 PM, Richard Earnshaw <rearnsha@arm.com> wrote: >>>> On 26/11/13 09:18, Eric Botcazou wrote: >>>>>> you are correct - this was an incorrect change. I believe that the >>>>>> patch below would be correct, but it is impossible to test it because (i >>>>>> believe) that gcc no longer works if the host_bits_per_wide_int is 32. >>>>>> I could be wrong about this but if i am correct, what do you want me to do? >>>>> >>>>> While you're right that most mainstream architectures now require a 64-bit >>>>> HWI, not all of them do according to config.gcc, so I don't think that this >>>>> path is entirely dead yet. I'll carry out the testing once we agree on the >>>>> final change. >>>> >>>> I'm hoping, once this patch series is in that we might be able to >>>> migrate the ARM port back to supporting a 32-bit HWI. The driving >>>> factor behind the original switch was supporting 128-bit constants for >>>> Neon and these patches should resolve that. >>> >>> i?86 would be another candidate (if you don't build a compiler with -m64 >>> support). >> >> Not true for x86 since we have >> >> Variable >> HOST_WIDE_INT ix86_isa_flags = TARGET_64BIT_DEFAULT | >> TARGET_SUBTARGET_ISA_DEFAULT >> >> in i386.opt. We need more than 32 bits for ix86_isa_flags. > > Then that should be HOST_WIDEST_INT instead. > Also one function in i386.c generates less optimal results when HOST_WIDE_INT is 32-bit such that we were generating different outputs from the same input on x86 and on x86-64 with -m32.
On Tue, Nov 26, 2013 at 3:15 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Tue, Nov 26, 2013 at 6:03 AM, <pinskia@gmail.com> wrote: >> >> >>> On Nov 26, 2013, at 6:00 AM, "H.J. Lu" <hjl.tools@gmail.com> wrote: >>> >>> On Tue, Nov 26, 2013 at 5:55 AM, Richard Biener >>> <richard.guenther@gmail.com> wrote: >>>> On Tue, Nov 26, 2013 at 2:44 PM, Richard Earnshaw <rearnsha@arm.com> wrote: >>>>> On 26/11/13 09:18, Eric Botcazou wrote: >>>>>>> you are correct - this was an incorrect change. I believe that the >>>>>>> patch below would be correct, but it is impossible to test it because (i >>>>>>> believe) that gcc no longer works if the host_bits_per_wide_int is 32. >>>>>>> I could be wrong about this but if i am correct, what do you want me to do? >>>>>> >>>>>> While you're right that most mainstream architectures now require a 64-bit >>>>>> HWI, not all of them do according to config.gcc, so I don't think that this >>>>>> path is entirely dead yet. I'll carry out the testing once we agree on the >>>>>> final change. >>>>> >>>>> I'm hoping, once this patch series is in that we might be able to >>>>> migrate the ARM port back to supporting a 32-bit HWI. The driving >>>>> factor behind the original switch was supporting 128-bit constants for >>>>> Neon and these patches should resolve that. >>>> >>>> i?86 would be another candidate (if you don't build a compiler with -m64 >>>> support). >>> >>> Not true for x86 since we have >>> >>> Variable >>> HOST_WIDE_INT ix86_isa_flags = TARGET_64BIT_DEFAULT | >>> TARGET_SUBTARGET_ISA_DEFAULT >>> >>> in i386.opt. We need more than 32 bits for ix86_isa_flags. >> >> Then that should be HOST_WIDEST_INT instead. >> > > Also one function in i386.c generates less optimal > results when HOST_WIDE_INT is 32-bit such that > we were generating different outputs from the same > input on x86 and on x86-64 with -m32. Well - we knew the code would bitrot once we decided to always default to 64bit HWI ... Richard. > -- > H.J.
On 11/26/2013 09:12 AM, Richard Biener wrote: > On Tue, Nov 26, 2013 at 3:00 PM, Kenneth Zadeck > <zadeck@naturalbridge.com> wrote: >> On 11/26/2013 08:44 AM, Richard Earnshaw wrote: >>> On 26/11/13 09:18, Eric Botcazou wrote: >>>>> you are correct - this was an incorrect change. I believe that the >>>>> patch below would be correct, but it is impossible to test it because (i >>>>> believe) that gcc no longer works if the host_bits_per_wide_int is 32. >>>>> I could be wrong about this but if i am correct, what do you want me to >>>>> do? >>>> While you're right that most mainstream architectures now require a >>>> 64-bit >>>> HWI, not all of them do according to config.gcc, so I don't think that >>>> this >>>> path is entirely dead yet. I'll carry out the testing once we agree on >>>> the >>>> final change. >>> I'm hoping, once this patch series is in that we might be able to >>> migrate the ARM port back to supporting a 32-bit HWI. The driving >>> factor behind the original switch was supporting 128-bit constants for >>> Neon and these patches should resolve that. >>> >>> R. >> >> Richard, >> >> I had not realized that you were into self abuse like that. you are >> going to have a bad time. I tried this as a way to test the wide-int branch >> because if we made hwi be 32bits, then it would trigger the long version of >> the implementation wide-int routines. What a disaster!!!! richard sandiford >> told me not to try and i ignored him. that was a wasted weekend!!! There >> are roadblocks everywhere. i certainly do not have a complete list, >> because i gave up after hacking a few things back. But very quickly you >> find places like real.[ch] which no longer work because the math that the >> dfp people hacked into it no longer works if the hwis are 32 bits. it is >> misleading because all of the tests are still there, it is just that the >> equations return the wrong answers. It is like this, one place after >> another. >> >> As far as your hope that once this patch goes in you would be able to >> support 128 bit constants is only partially true. it will/should be the >> case, that you always get the correct answer and the code will not ice. >> That is the good news. But the bad news is that there is still way to >> much code of the form, if var fits in hwi, do some optimization, else do not >> do the optimization. Richi did not want me to attack this code yet because >> it would have made the patches even bigger. For the rtl level a lot of >> this was removed because i did it before he told me not to, but the tree >> level still has tons of code like this. So at least for a long time, like >> the close of the next release, this is not going to get fixed. > Note that in the long run we want wide-int to use HOST_WIDEST_FAST_INT > and not HOST_WIDE_INT internally. I know I asked you to do that > but it's of course too late now. > > Richard. i started to do it and it turned out to be harder than either of us thought it would. As i remember it, the problem came up of what were you going to populate the tree_csts and const_wide_ints. I assume that you wanted those to have HWFI in them also. Then i started looking at the *fits_*hwi to_*hwi which is used in a billion places and that was now starting to look expensive. perhaps this change could/should be done later after there are almost no calls to these functions. But while there are still a billion of them in the code, this seemed like a mistake. >> Kenny >> >> >>
On 11/26/2013 09:16 AM, Richard Biener wrote: > On Tue, Nov 26, 2013 at 3:15 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Tue, Nov 26, 2013 at 6:03 AM, <pinskia@gmail.com> wrote: >>> >>>> On Nov 26, 2013, at 6:00 AM, "H.J. Lu" <hjl.tools@gmail.com> wrote: >>>> >>>> On Tue, Nov 26, 2013 at 5:55 AM, Richard Biener >>>> <richard.guenther@gmail.com> wrote: >>>>> On Tue, Nov 26, 2013 at 2:44 PM, Richard Earnshaw <rearnsha@arm.com> wrote: >>>>>> On 26/11/13 09:18, Eric Botcazou wrote: >>>>>>>> you are correct - this was an incorrect change. I believe that the >>>>>>>> patch below would be correct, but it is impossible to test it because (i >>>>>>>> believe) that gcc no longer works if the host_bits_per_wide_int is 32. >>>>>>>> I could be wrong about this but if i am correct, what do you want me to do? >>>>>>> While you're right that most mainstream architectures now require a 64-bit >>>>>>> HWI, not all of them do according to config.gcc, so I don't think that this >>>>>>> path is entirely dead yet. I'll carry out the testing once we agree on the >>>>>>> final change. >>>>>> I'm hoping, once this patch series is in that we might be able to >>>>>> migrate the ARM port back to supporting a 32-bit HWI. The driving >>>>>> factor behind the original switch was supporting 128-bit constants for >>>>>> Neon and these patches should resolve that. >>>>> i?86 would be another candidate (if you don't build a compiler with -m64 >>>>> support). >>>> Not true for x86 since we have >>>> >>>> Variable >>>> HOST_WIDE_INT ix86_isa_flags = TARGET_64BIT_DEFAULT | >>>> TARGET_SUBTARGET_ISA_DEFAULT >>>> >>>> in i386.opt. We need more than 32 bits for ix86_isa_flags. >>> Then that should be HOST_WIDEST_INT instead. >>> >> Also one function in i386.c generates less optimal >> results when HOST_WIDE_INT is 32-bit such that >> we were generating different outputs from the same >> input on x86 and on x86-64 with -m32. > Well - we knew the code would bitrot once we decided to always > default to 64bit HWI ... > > Richard. > >> -- >> H.J. Just for the record, the straw the broke the camel's back for me was some bit map data structure in one of the gen functions that has a HWI worth of bits and i needed 35 of them for the port i was trying. It was all fixable, but i had already signed up for one task that seemed like it was going to take the rest of my like and i did not need another. kenny
> I had not realized that you were into self abuse like that. you are > going to have a bad time. I tried this as a way to test the wide-int > branch because if we made hwi be 32bits, then it would trigger the long > version of the implementation wide-int routines. What a disaster!!!! > richard sandiford told me not to try and i ignored him. that was a > wasted weekend!!! There are roadblocks everywhere. i certainly do > not have a complete list, because i gave up after hacking a few things > back. But very quickly you find places like real.[ch] which no longer > work because the math that the dfp people hacked into it no longer works > if the hwis are 32 bits. it is misleading because all of the tests > are still there, it is just that the equations return the wrong > answers. It is like this, one place after another. I think that you're exaggerating here, there might be some breakages but they should be localized and fixable. We have 4.7-based 32-bit HWI cross-compilers and they work pretty well, at least Ada and C, and I don't think that anything has substantially changed on the mainline.
Index: gcc/ada/gcc-interface/cuintp.c =================================================================== --- gcc/ada/gcc-interface/cuintp.c (revision 205374) +++ gcc/ada/gcc-interface/cuintp.c (working copy) @@ -167,7 +167,7 @@ UI_From_gnu (tree Input) in a signed 64-bit integer. */ if (tree_fits_shwi_p (Input)) return UI_From_Int (tree_to_shwi (Input)); - else if (wi::lts_p (Input, 0) && TYPE_UNSIGNED (gnu_type)) + else if (wi::gtu_p (wi::lrshift (Input, 63), 0) && TYPE_UNSIGNED (gnu_type)) return No_Uint; #endif