[PATCH] MachineSink: Fix and tweak critical-edge breaking heuristic.

Will Dietz w at wdtz.org
Sat Sep 7 20:51:57 PDT 2013


See attached, introduction is below and a description from commit
message is repeated at the end of this message for easier processing
and discussion.

This fixes a serious compile-time performance issue for me and hopefully
causes LLVM to generate slightly nicer code.  Unfortunately I'm not in
a position
to do appropriate performance or platform testing of this (perhaps
minor) change, so please let me know how I can best proceed or if
that's something
to leave to nightly testing after the change is finished and passes review.

Unfortunately, this breaks a few lit tests in ways that aren't
obviously problematic upon inspection, but it's unclear to me how to
properly resolve them.  The tests in question are:

    LLVM :: CodeGen/ARM/2011-04-11-MachineLICMBug.ll
    LLVM :: CodeGen/ARM/2011-08-25-ldmia_ret.ll
    LLVM :: CodeGen/ARM/2012-08-30-select.ll
    LLVM :: CodeGen/X86/2012-11-30-handlemove-dbg.ll
    LLVM :: CodeGen/X86/fold-load.ll
    LLVM :: CodeGen/X86/hoist-common.ll

Can someone more familiar with these help me work through them?

A self-hosted copy of LLVM/Clang with this change passes its tests
(other than the ones above) and seems to build various applications
properly, so
I'm hopeful these just need to be updated to accommodate this change
and aren't signs of regressions or correctness issues.

Please let me know if you have any questions/comments/feedback, and
thank you for your time.

--------------------------------------------------------

Per original comment, the intention of this loop
is to go ahead and break the critical edge
(in order to sink this instruction) if there's
reason to believe doing so might "unblock" the
sinking of additional instructions that define
registers used by this one.  The idea is that if
we have a few instructions to sink "together"
breaking the edge might be worthwhile.

This commit makes a few small changes
to help better realize this goal:

First, modify the loop to ignore registers
defined by this instruction.  We don't
sink definitions of physical registers,
and sinking an SSA definition isn't
going to unblock an upstream instruction.

Second, ignore uses of physical registers.
Instructions that define physical registers are
rejected for sinking, and so moving this one
won't enable moving any defining instructions.
As an added bonus, while virtual register
use-def chains are generally small due
to SSA goodness, iteration over the uses
and definitions (used by hasOneNonDBGUse)
for physical registers like EFLAGS
can be rather expensive in practice.
(This is the original reason for looking at this)

Finally, to keep things simple continue
to only consider this trick for registers that
have a single use (via hasOneNonDBGUse),
but to avoid spuriously breaking critical edges
only do so if the definition resides
in the same MBB and therefore this one directly
blocks it from being sunk as well.
If sinking them together is meant to be,
let the iterative nature of this pass
sink the definition into this block first.

~Will
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-MachineSink-Fix-and-tweak-critical-edge-breaking-heu.patch
Type: application/octet-stream
Size: 4204 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130907/e095991e/attachment.obj>


More information about the llvm-commits mailing list