On 05/22/2015 09:45 AM, Segher Boessenkool wrote:
On Thu, May 21, 2015 at 09:24:37AM -0600, Jeff Law wrote:
When combine needs to split a complex insn, it will canonicalize a
simple (mult X (const_int Y)) where Y is a power of 2 into the expected
(ashift X (const_int Y')) if the (mult ...) is selected as a split point.
However if the split point is (plus (mult (X (const_int Y)) Z) combine
fails to canonicalize.
OK for the trunk?
Okay. Something went wrong with your changelog though. Rebasing
changelogs is a recipe for disaster.
Yup. I still haven't found a workflow that makes sense with ChangeLogs.
Ultimately I end up having to check them before each commit/push, so
assume it'll end up in the right place :-)
The following is just copy-paste, but anyway...
+ /* Similarly for (plus (mult FOO (const_int pow2))). */
+ if (split_code == PLUS
+ && GET_CODE (XEXP (*split, 0)) == MULT
+ && CONST_INT_P (XEXP (XEXP (*split, 0), 1))
+ && INTVAL (XEXP (XEXP (*split, 0), 1)) > 0
+ && (i = exact_log2 (UINTVAL (XEXP (XEXP (*split, 0), 1)))) >= 0)
INTVAL (..) > 0 here disallows 1<<63 while exact_log2 allows it
(with a 64-bit HWI). Most places don't check the > 0 and it doesn't
seem necessary in the places that do.
This won't ever trigger for a SImode 1<<31 either.
None of this matters when we're just looking at scaled indexing, of
course. But exact_log2 is much harder to use correctly than to use
it incorrectly :-(
That test was just copied from the equivalent hunk of code that tests
for (mult FOO (const_int pow2)) above. It's certainly ok for the INTVAL
to reject things that might be allowed through the exact_log2 test. But
as you say, this isn't going to matter.
Jeff
