This is an archive of the discontinued LLVM Phabricator instance.

Fix assertion in FunctionComparator::cmpInlineAsm
ClosedPublic

Authored by nikic on May 5 2018, 11:11 AM.

Details

Summary

Fixes bug https://bugs.llvm.org/show_bug.cgi?id=37339.

InlineAsm is only uniqued if the FunctionTypes are exactly the
same, while cmpTypes() for example considers all pointer types
in the default address space to be the same. For this reason
the end of cmpInlineAsm() can be reached.

This patch replaces the unreachable assertion with a check that
the function types are not identical.

Diff Detail

Event Timeline

nikic created this revision.May 5 2018, 11:11 AM
nox added a subscriber: nox.May 5 2018, 1:18 PM
jfb added inline comments.May 7 2018, 10:10 PM
test/Transforms/MergeFunc/inline-asm.ll
6

Can you add a separate test where the function types are the same, but the targets are different (i.e. not null)?

jfb requested changes to this revision.May 7 2018, 10:10 PM
This revision now requires changes to proceed.May 7 2018, 10:10 PM
nikic updated this revision to Diff 145888.May 9 2018, 3:04 AM

Updated test to test both different pointer types as inline asm argument types and return types.

I've also adjusted the test to make the functions internal and have a separate function call them. This avoids the need for the somewhat non-obvious call void @stuff calls to satisfy minimum function size.

nikic updated this revision to Diff 145892.May 9 2018, 3:37 AM

Sorry, I misunderstood your previous comment and thought you were referring to the return type. I've added two variants to the test now, one where a non-null argument is used and it can be merged and one where it can't be merged.

jfb accepted this revision.May 9 2018, 9:03 AM
This revision is now accepted and ready to land.May 9 2018, 9:03 AM
nox added a comment.May 10 2018, 7:39 AM

Can anyone land that?

This revision was automatically updated to reflect the committed changes.