Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#7116 closed bug (fixed)

Missing optimisation: strength reduction of floating-point multiplication

Reported by: simonmar Owned by: Jan Stolarek <jan.stolarek@…>
Priority: normal Milestone: 7.8.1
Component: Compiler Version: 7.4.2
Keywords: Cc: jan.stolarek@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case: numeric/should_compile/T7116
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

e.g. x * 2.0 should turn into x + x. See #2253 (program 6) for an example.

Whether this is best done as a builtin RULE or in cmmMachOpFold, or possibly both, I don't know.

Attachments (1)

7116.patch (4.8 KB) - added by pcapriotti 5 years ago.

Download all attachments as: .zip

Change History (13)

Changed 5 years ago by pcapriotti

Attachment: 7116.patch added

comment:1 Changed 5 years ago by pcapriotti

I think this should be in PrelRules. Attached is a patch. Could it be done more generally?

comment:2 Changed 5 years ago by simonmar

Looks good to me.

comment:3 Changed 5 years ago by simonpj

Owner: set to pcapriotti

Paolo: yes, please commit your patch. But could you add a comment Note [Strength reduction] that explains how it works. In particular the "lit" is always 2, and the op is always (+); but at either Float or Double. Perhapas you can name the op parameter add_op? And lit can be two_lit. But the comment as well!

Thanks

Simon

comment:4 Changed 4 years ago by jstolarek

Provded patch rewrites 2 * x for Int and Word to x + x, which kills the optimisation done at the later stage of the pipeline in CmmOpt: turning multiplications and divisons by powers of two into shifts. I wonder if it makes sense to move mentioned Cmm optimisation to PrelRules, so that shifts are introduced at the Core level. Having all similar optimisations done in one place would be more consistent. Or am I missing something?

comment:5 Changed 4 years ago by igloo

Status: newpatch

comment:6 Changed 4 years ago by igloo

Owner: pcapriotti deleted

comment:7 Changed 4 years ago by simonmar

jstolarek: the ticket is about doing the optimisation for floating-point multiplication, not integer. We don't want to do it for integer operations, because it's better to do this in the code generator (the only reason for doing it in the simplifier would be to enable further simplifications, but that isn't the case here).

Furthermore, we definitely want to optimise integer multiplication at the Cmm level, because the Stg->Cmm code generator and later stages may create opportunities for this optimisation.

comment:8 Changed 4 years ago by jstolarek

Cc: jan.stolarek@… added

comment:9 Changed 4 years ago by jan.stolarek@…

commit 838e2fda9892e61da85187803cce45b3f815b86e

Author: Jan Stolarek <jan.stolarek@p.lodz.pl>
Date:   Mon Jul 8 16:20:43 2013 +0100

    Add strength reduction rules (Fixes #7116)
    
    This patch adds rules for converting floating point multiplication
    of the form 2.0 * x and x * 2.0 into addition x + x.

 compiler/prelude/PrelRules.lhs |   33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

comment:10 Changed 4 years ago by jstolarek

Resolution: fixed
Status: patchclosed

I pushed fix into HEAD.

Really sorry about the merge commit - the moment I pushed I realized that I didn't pull with --rebase.

comment:11 Changed 4 years ago by Jan Stolarek <jan.stolarek@…>

Owner: set to Jan Stolarek <jan.stolarek@…>

In 67d6a196d299baa99b11dd6be82472b91129618d/testsuite:

Fix #7116 failing due to uniques

comment:12 Changed 4 years ago by jstolarek

Test Case: numeric/should_compile/T7116
Note: See TracTickets for help on using tickets.