On Mar 26, 2014, at 11:33 AM, Jeff Law <l...@redhat.com> wrote: > On 03/26/14 12:28, Jakub Jelinek wrote: >> On Wed, Mar 26, 2014 at 12:17:43PM -0600, Jeff Law wrote: >>> On 03/26/14 12:12, Jakub Jelinek wrote: >>>> On Wed, Mar 26, 2014 at 11:02:48AM -0600, Jeff Law wrote: >>>>> Bootstrapped and regression tested on x86_64-unknown-linux-gnu. >>>>> Verified it fixes the original and reduced testcase. >>>> >>>> Note, the testcase is missing from your patch. >>>> >>>> But I'd question if this is the right place to canonicalize it. >>>> The non-canonical order seems to be created in the generic code, where >>>> do_tablejump does: >>> No, at that point it's still canonical because the x86 backend >>> hasn't simpified the (mult ...) subexpression. Its the >>> simplification of that subexpression to a constant that creates the >>> non-canonical RTL. That's why I fixed the x86 bits -- those are the >>> bits that simplify the (mult ...) into a (const_int) and thus >>> creates the non-canonical RTL. >> >> (mult:SI (const_int 0) (const_int 4)) is IMHO far from being canonical. > It's debatable.
(set (reg) (plus (reg) (mult 2 4))) should be canonicalized into (set (reg) (plus (reg) 8)) remember, the entire point of canonicalization is so that the port doesn’t have to match: (set (reg) (plus (reg) (mult 2 4))) and (set (reg) (plus (reg) 8) with two patterns to get good code-gen. I know, we goof the rules from time to time, but when we do, we should just admit there is no value to it, and fix it. Anyway, the documented rule is: > There are often cases where multiple RTL expressions could represent an > operation performed by a single machine instruction. This situation is > most commonly encountered with logical, branch, and multiply-accumulate > instructions. In such cases, the compiler attempts to convert these > multiple RTL expressions into a single canonical form to reduce the > number of insn patterns required. > > In addition to algebraic simplifications, Now, when look up algebraic simplification, I get: http://blog.wolframalpha.com/2011/04/25/algebraic-simplification-simplifying-expressions-in-wolframalpha/ and when I look up: http://www.wolframalpha.com/input/?i=2+*+4 it says the answer is 8. If that isn’t the right answer, please fix google and/or wolfram alpha as one of them is seriously misleading. If the documented rule is wrong, please fix the document. ? > Our canonicalization rules don't explicitly cover the case where both > arguments to a commutative expression are constants. Then why do we document it under: @section Canonicalization of Instructions @cindex canonicalization of instructions @cindex insn canonicalization ?
