This is an archive of the discontinued LLVM Phabricator instance.

[PGO][indirect-call-promotion] Add extra parameter check for musttail callsite
ClosedPublic

Authored by xur on Nov 30 2022, 3:26 PM.

Details

Summary

Add extra parameter type check to musttail callsite to avoid verifier error.

Diff Detail

Event Timeline

xur created this revision.Nov 30 2022, 3:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 30 2022, 3:26 PM
xur requested review of this revision.Nov 30 2022, 3:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 30 2022, 3:26 PM
rnk added inline comments.Nov 30 2022, 4:13 PM
llvm/test/Transforms/PGOProfile/indirect_call_promotion_musttail_typecheck.ll
30

I would like to see more of the successfully transformed IR, I want to see if the musttail call is in the tail position, since that's also a verifier requirement.

Can we use https://github.com/llvm/llvm-project/blob/main/llvm/utils/update_test_checks.py for this test? There's not too much extraneous IR.

xur updated this revision to Diff 479177.Nov 30 2022, 11:32 PM

Followed Reid's suggestion to explicitly check the IR after the pass.

tejohnson accepted this revision.Dec 1 2022, 6:24 AM

lgtm with a couple comment suggestions for test

llvm/test/Transforms/PGOProfile/indirect_call_promotion_musttail_typecheck.ll
13

Add comment that we are checking that there was no icp due to parameter mismatch.

34

Add comment that we are checking that icp succeeded since the parameter types match and that the promoted call is also musttail.

This revision is now accepted and ready to land.Dec 1 2022, 6:24 AM
rnk accepted this revision.Dec 1 2022, 10:32 AM

Thanks, my concerns are addressed!

xur marked 2 inline comments as done.Dec 2 2022, 9:53 AM
xur updated this revision to Diff 479666.Dec 2 2022, 9:56 AM

Add comments in the tests suggested by Teresa.

This revision was landed with ongoing or failed builds.Dec 2 2022, 10:52 AM
This revision was automatically updated to reflect the committed changes.