https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119872
--- Comment #10 from Tamar Christina <tnfchris at gcc dot gnu.org> --- (In reply to rguent...@suse.de from comment #9) > On Mon, 21 Apr 2025, tnfchris at gcc dot gnu.org wrote: > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119872 > > > > --- Comment #8 from Tamar Christina <tnfchris at gcc dot gnu.org> --- > > (In reply to Richard Biener from comment #7) > > > Please make sure to not "fix" something where the input is already wrong - > > > see the various issues where SCEV produces an invalid CHREC - forming a > > > chrec > > > is association. > > > > > > Using twice wide types within the same context is wrong IMO, the intent of > > > the previous change was to identify IVs that end up with the same sequence > > > of IV values, and we should preserve that. I don't see whether this is > > > actually a case where the IV values are not the same? It seems we're > > > using > > > one in place of the other in a context where the evolution expression > > > itself > > > is re-used (but the one with undefined overflow) and that's the wrong > > > thing > > > to do? > > > > That's fair I was operating under the impression the original code had a > > reason > > to do what it did. > > > > The more I look at it the more I'm not sure why it generated incorrect code > > as > > at most I would have expected it to use another IV. So will look into that. > > > > That said the original constant_multiple_of is quite odd.. > > > > e.g. > > > > (rr) p debug (top) > > 4294967292 > > as signed int this is -4 > > > $7 = void > > (rr) p debug (bot) > > 4294967295 > > and this -1 > > > > > for non-poly it does > > > > case INTEGER_CST: > > if (TREE_CODE (bot) != INTEGER_CST) > > return false; > > > > p0 = widest_int::from (wi::to_wide (top), SIGNED); > > p1 = widest_int::from (wi::to_wide (bot), SIGNED); > > if (p1 == 0) > > return false; > > *mul = wi::sext (wi::divmod_trunc (p0, p1, SIGNED, &res), precision); > > > > > > so it would return that those two are a multiple of one another and that > > mul == > > 4 which clearly is bogus if it's supposed to check multiples. > > which makes this look correct. Also > > (gdb) p (unsigned int)(4294967295 * 4) > $3 = 4294967292 > > so even for unsigned it's correct. Note this relies on 'SIGNED' > intepretation, otherwise the division/modulo results are "off". > > > for Poly it does what's expected: > > > > if (POLY_INT_CST_P (top) > > && POLY_INT_CST_P (bot) > > && constant_multiple_p (wi::to_poly_widest (top), > > wi::to_poly_widest (bot), mul)) > > return true; > > > > So this code is odd.. > > The poly implementation looks quite similar to me, how does it > arrive at a different result? Because wi::to_poly_widest uses > TYPE_SIGN and the modulo is thus computed non-zero? Yeah that's my fault.. I forgot to consider this can be a signed int printed as unsigned. and when I tested the values I forgot the cast. So that's all fine. I'll focus instead on why not sharing the IV resulted in incorrect code rather than why the IV wasn't shared. Sorry for the noise.
