This is an archive of the discontinued LLVM Phabricator instance.

MergeFunc: Don't merge functions with different range metadata on call/invoke
ClosedPublic

Authored by dotdash on Jun 22 2014, 6:11 AM.

Details

Summary

Since r211281 call and invoke can have range metadata which must be
taken into consideration when comparing such instructions.

Diff Detail

Event Timeline

dotdash updated this revision to Diff 10730.Jun 22 2014, 6:11 AM
dotdash retitled this revision from to MergeFunc: Don't merge functions with different range metadata on call/invoke.
dotdash updated this object.
dotdash edited the test plan for this revision. (Show Details)
dotdash added a reviewer: dyatkovskiy.
dotdash added a subscriber: Unknown Object (MLST).
dyatkovskiy edited edge metadata.EditedJun 23 2014, 7:07 PM

Hi Björn,

The changes itself looks fine, but I have to check tests as well.. I'm moving on this week, so I can answer you in Wednesday only.
Thanks for working on it.
By the way, we finished our O(n*log(n)) patch series. You could look into FunctionPtr. Perhaps it had to be renamed to FunctionInfo or something like this.
Anyways in this structure you could keep additional data for instructions you need (such as ranges that were merged currently). I try to think, how to bring it into most generic form. Perhaps that's not only about ranges and tbaa..

Thanks!

Changes in MergeFunctions.cpp - LGTM
call-with-ranges.ll - LGTM
invoke-with-ranges.ll - read inline comment, otherwise - fine.

test/Transforms/MergeFunc/invoke-with-ranges.ll
11

Can we avoid bitcasts?

dotdash updated this revision to Diff 10878.Jun 25 2014, 11:38 PM
dotdash edited edge metadata.

Updated the tests for invoke-with-ranges to avoid the bitcasts

dyatkovskiy added a reviewer: nicholas.EditedJun 25 2014, 11:39 PM

LGTM,
now let's wait Nick and talk to him :-)

Hi Nick,
can I commit patch for Björn?
Thanks!
-Stepan

Stepan Dyatkovskiy wrote:

LGTM

http://reviews.llvm.org/D4246

ping
Stepan Dyatkovskiy wrote:

Hi Nick,
can I commit patch for Björn?
Thanks!
-Stepan

Stepan Dyatkovskiy wrote:

LGTM

http://reviews.llvm.org/D4246

nicholas accepted this revision.Jul 11 2014, 12:09 PM
nicholas edited edge metadata.

Optional: I think the two tests could be merged into one file. Either way, LGTM.

This revision is now accepted and ready to land.Jul 11 2014, 12:09 PM
dyatkovskiy closed this revision.Jul 15 2014, 11:59 AM

Committed as r213060
Thanks for working on it!