This is an archive of the discontinued LLVM Phabricator instance.

Preserve metadata on masked intrinsics in auto-upgrade
ClosedPublic

Authored by kparzysz on Apr 23 2021, 1:55 PM.

Details

Summary

When auto-upgrade was replacing a call to a masked intrinsic, it would not copy the metadata from the original call.

If an intrinsic had metadata, but did not need any updates, the metadata would stay, but if an update was needed, the metadata would end up being removed. A similar effect could be observed with masked_expandload and masked_compressstore, which at the moment are not handled by auto-upgrade: the metadata remained untouched.

There are likely other places where metadata is not preserved on masked intrinsics, those will be fixed separately. It seems like there hasn't been any expectation of having specifically TBAA metadata information on intrinsics, but having them there can definitely be helpful (we create them in a project that uses LLVM as backend, and they do help with alias analysis).

Diff Detail

Event Timeline

kparzysz created this revision.Apr 23 2021, 1:55 PM
kparzysz requested review of this revision.Apr 23 2021, 1:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 23 2021, 1:56 PM

Ping. Could someone weigh in on this?

fhahn added inline comments.Apr 30 2021, 8:15 AM
llvm/lib/IR/AutoUpgrade.cpp
3900

this copies any metadata, right?

I think it makes sense to keep any metadata in this case, but it would be good to update the title & description to make this clear. And also extend the tests with a few different metadata kinds.

kparzysz updated this revision to Diff 341982.Apr 30 2021, 11:01 AM
kparzysz retitled this revision from Preserve TBAA metadata on masked intrinsics in auto-upgrade to Preserve metadata on masked intrinsics in auto-upgrade.
kparzysz edited the summary of this revision. (Show Details)

Added debug metadata to the testcase. Updated summary to refer to metadata in general, not specifically TBAA (although TBAA was mentioned in one specific context).

kparzysz marked an inline comment as done.Apr 30 2021, 11:01 AM

Ping. Hopefully this is ready to go.

Looks good to me. Thanks.

This revision is now accepted and ready to land.May 5 2021, 11:56 AM
This revision was automatically updated to reflect the committed changes.
fhahn added a comment.May 5 2021, 1:53 PM

Thanks for the updates! LGTM :)