On Jan 23, 2015, at 3:03 AM, Tejas Belagod <tejas.bela...@arm.com> wrote: > This is an almost obvious patch to fix PR64231 as discovered by A. Pinksi and > as proposed by Jakub.
Kinda crappy code. The macro to use here should take the number of bits as an int, and wether the constant is signed or not. FITS (x, 32, UNSIGNED) would be nicer. Complexity of your patch, 16, complexity of the above, 4. This is 4x better. Additionally, since more than one port and more than 1 place in the compiler does this, would be nice to refactor them all to be as nice. I’m sore on this topic, as I’ve seen failing code that used the style in your patch that I had to fix in GNU ld. It was as annoying to find and fix as this PR. Now what are the odds that someone doesn’t understand how to use IN_RANGE and does this same exact problem again, multiplied by the number of times IN_RANGE appears in gcc multiplied by the time it took for everyone to track down and talk about this bug? Compare than number to 0. In my version, it should be impossible to ever misuse, leading to a bug rate of near 0. The biggest issue with mine, one will need to add overloads for tree, rtx, int, long, maxint_t as the code is found to not compile. These, while annoying, are much easier to resolve than the bug in the PR.
