Page MenuHomePhabricator

Add test for D21736 + D21737.
ClosedPublic

Authored by koriakin on Jun 26 2016, 7:22 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

vsk accepted this revision.Jun 27 2016, 8:22 AM
vsk edited edge metadata.

LGTM, with one nit.

test/Profile/cxx-indirect-call.cpp
3 ↗(On Diff #61922)

I don't really understand the significance of makeItaniumABITriple. Could you explain why this change is necessary? If it's not, please drop it.

This revision is now accepted and ready to land.Jun 27 2016, 8:22 AM
koriakin added inline comments.Jun 27 2016, 8:49 AM
test/Profile/cxx-indirect-call.cpp
3 ↗(On Diff #61922)

%itanium_abi_tuple may give any target triple that's not win32 - which includes s390x and other platforms that need zeroext. Since this test assumes __llvm_profile_instrument_target has last parameter without zeroext, it'd break when run on s390x. Hardwiring the target to x86_64 ensures we'll never emit zeroext here.

vsk added a comment.Jun 27 2016, 9:02 AM

Is the plan to commit this along with D21737?

test/Profile/cxx-indirect-call.cpp
3 ↗(On Diff #61922)

Ok! Thanks for explaining.

In D21741#467969, @vsk wrote:

Is the plan to commit this along with D21737?

Yes, and with D21736+D21739.

koriakin retitled this revision from Add test for D21736 and D21737. to Add test for D21736..Jun 27 2016, 5:00 PM
koriakin edited edge metadata.
koriakin retitled this revision from Add test for D21736. to Add test for D21736 + D21737..Oct 23 2016, 12:08 PM
This revision was automatically updated to reflect the committed changes.