This is an archive of the discontinued LLVM Phabricator instance.

[JumpThreading] Fix opcode bonus in getJumpThreadDuplicationCost()
ClosedPublic

Authored by gberry on Dec 15 2015, 10:57 AM.

Details

Summary

The code that was meant to adjust the duplication cost based on the
terminator opcode was not being executed in cases where the initial
threshold was hit inside the loop.

Diff Detail

Repository
rL LLVM

Event Timeline

gberry updated this revision to Diff 42879.Dec 15 2015, 10:57 AM
gberry retitled this revision from to [JumpThreading] Fix opcode bonus in getJumpThreadDuplicationCost().
gberry updated this object.
gberry added subscribers: llvm-commits, mcrosier.
gberry updated this revision to Diff 42881.Dec 15 2015, 11:22 AM

Updated description

gberry updated this object.Dec 15 2015, 11:22 AM

Nadav, could you review this? It appears that a change of yours introduced this change in behavior:

c6990869 (Nadav Rotem 2012-12-03 17:34:44 +0000 294) // Stop scanning the block if we've reached the threshold.
c6990869 (Nadav Rotem 2012-12-03 17:34:44 +0000 295) if (Size > Threshold)
c6990869 (Nadav Rotem 2012-12-03 17:34:44 +0000 296) return Size;

Benchmarking results (on AArch64 A57-like processor):

spec2006/bzip2: improves 1.7%
no other significant changes in spec2000/2006

testsuite:
no significant changes
clamscan has similar static code changes as spec2006/bzip2 (it looks like it has a copy bzip2 in it), but these changes don't effect runtime performance

mcrosier accepted this revision.Dec 29 2015, 8:35 AM
mcrosier added a reviewer: mcrosier.

This looks like a straight forward fix to unintended behavior. LGTM with one minor suggestion.

lib/Transforms/Scalar/JumpThreading.cpp
346 ↗(On Diff #42881)

How about just:

return Size > Bonus ? Size - Bonus : 0;

This revision is now accepted and ready to land.Dec 29 2015, 8:35 AM
gberry marked an inline comment as done.Dec 29 2015, 8:55 AM

Will do, thanks.

This revision was automatically updated to reflect the committed changes.