On Wed, Oct 17, 2012 at 11:47 PM, Kenneth Zadeck <zad...@naturalbridge.com> wrote: > Richi, > > I apologize for the size of this patch, but it only does one very small > thing, it is just that it does it all over the middle end. > > This patch introduces a new api for extracting a signed or unsigned hwi from > an integer cst. There had been an abi for doing this, but it has some > problems that were fixed here, and it was only used sometimes. > > The new abi consists of 6 functions, three for testing if the constant in > the int cst will fit and three for actually pulling out the value. > > The ones for testing are tree_fits_uhwi_p, tree_fits_shwi_p, > tree_fits_hwi_p. The first two of these are unsigned and signed versions, > and the second one takes a boolean parameter which is true if the value is > positive. This replaces the host_integerp which basically has the same > functionality of tree_fits_hwi_p. The reason for changing this is that > there were about 400 calls to host_integerp and all but 4 of them took a > constant parameter. These names are chosen to be similar to the similar > functions in wide-int and are more mnemonic that the existing name since > they use the more readable u and s prefixes that a lot of other places do. > > On the accessing side, there is tree_to_uhwi, tree_to_shwi, and tree_to_hwi. > The first two do checking when checking is enabled. The last does no > checking.
Just a quick note here - the changelog mentions tree_low_cst (as new function!?) but not host_integerp. You should probably get rid of both, otherwise uses will creap back as people are familiar with them (I'm not sure these changes for consistency are always good ...) I don't like that tree_to_hwi does not do any checking. In fact I don't like that it _exists_, after all it has a return type which signedness does not magically change. Unchecked conversions should use TREE_LOW_CST. Thus, my 2 cents before I really look at the patch (which will likely be next week only, so maybe you can do a followup with the above suggestions). Thanks, Richard. > it is expected normally, the unchecked accesses follows an explicit check, > or if there is no explicit check, then one of the checked ones follows. > This is always not the case for places that just want to look at the bottom > bits of some large number, such as several places that compute alignment. > These just use the naked unchecked access. > > There were a lot of places that either did no checking or did the checking > inline. This patch tracks down all of those places and they now use the > same abi. > > There are three places where i found what appear to be bugs in the code. > These are places where the existing code did an unsigned check and then > followed it with a signed access (or visa versa). These places are marked > with comments that contain the string "THE NEXT LINE IS POSSIBLY WRONG". > With some guidance from the reviewer, i will fix these are remove the > unacceptable comments. > > Aside from that, this patch is very very boring because it just makes this > one transformation. Look for interesting stuff in tree.h. Everything else > is just forcing everywhere to use a single abi. > > This patch could go on with a little work without the other 5 patches or it > can wait for them. > > kenny
