This is an archive of the discontinued LLVM Phabricator instance.

Preserve GlobalsAA analysis result in LowerConstantIntrinsics
ClosedPublic

Authored by rsanthir.quic on Jun 22 2020, 7:19 PM.

Details

Summary

LowerConstantIntrinsics fails to preserve the analysis result of GlobalsAA. Not preserving the analysis might affect benchmark performance. This change fixes this issue.

Diff Detail

Event Timeline

rsanthir.quic created this revision.Jun 22 2020, 7:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 22 2020, 7:19 PM
fhahn accepted this revision.Jun 24 2020, 5:08 AM

LGTM. LowerConstantIntrinsics should not modify globals and can preserve GlobalsAA. This is also in line with what we currently do in many other passes and not preserving GlobalsAA can lead to serious performance regressions.

I can commit this patch for you in a day or so, to give other people a chance to chime in as well.

This revision is now accepted and ready to land.Jun 24 2020, 5:08 AM

@Florian since we've given it a couple days, would it be ok to make the commit now?

@Florian since we've given it a couple days, would it be ok to make the commit now?

@fhahn I think this was directed towards you.

fhahn added a comment.Jul 2 2020, 7:46 AM

@Florian since we've given it a couple days, would it be ok to make the commit now?

Thanks for the reminder! Committed as e6cf796bab7e02d2b8ac7fd495f14f5e21494270

This revision was automatically updated to reflect the committed changes.

My apologies @Florian! Thank you @fhahn!