On Sun, May 21, 2017 at 9:45 PM, Marc Glisse <marc.gli...@inria.fr> wrote: > Hello, > > when we have a double*p, computing p+n, we multiply n by 8 (size of double) > then add it to p. According to the comment just below the modified lines in > the attached patch, the multiplication is supposed to happen in a signed > type. However, that does not match the current code which uses sizetype. > This is causing missed optimizations, for instance, when comparing p+m and > p+n, we might end up comparing 8*m to 8*n, which is not the same as > comparing m and n if overflow can happen. > > I tried this patch to see how much breaks. And actually, not that much does. > There are a few fragile testcases that want to match "q_. " but it is now > "q_10 ", or they expect 3 simplifications and get 5 (same final code). One > was confused by a cast in the middle of x+cst1+cst2. A couple were hit by > the lack of CSE between (x+1)*8, x*8+8, and some variants with casts in the > middle, but that's already an issue without the patch. A few vectorization > testcases failed because SCEV did not recognize a simple increment of a > variable with NOP casts in the middle, that would be worth fixing. The patch > actually fixed another vectorization testcase. > > I guess it would help if pointer_plus switched to a signed second argument, > to be consistent with this and reduce the number of casts.
Yes. In fact the pointer-plus offset is to be interpreted as signed anyway so that it uses unsigned sizetype was a bad decision back in time. Bin tried to fix this last year but I'm not sure what came of his experiments. > I am not proposing the patch as is, but is this the direction we want to > follow, or should I just fix the comment to match what the code does? I think I tried this at some point and I don't remember the result. I did that in the context of the even more ambitious attempt to make pointer-plus take either sizetype or ssizetype typed offsets... The standard doesn't actually talk about what type to perform the multiplication in but given we effectively restrict objects to the size of half of the address space due to ptrdiff_t limitations using either a signed or unsigned type should work. Richard. > -- > Marc Glisse
