This is an archive of the discontinued LLVM Phabricator instance.

[SCEVExpander] Support opaque pointers
ClosedPublic

Authored by nikic on Jul 3 2021, 11:52 AM.

Details

Reviewers
dblaikie
Group Reviewers
Restricted Project
Commits
rG84c15bc018fa: [SCEVExpander] Support opaque pointers
Summary

This adds support for opaque pointers to expandAddToGEP() by always generating an i8 GEP for opaque pointers. After looking at some other cases (constexpr GEP folding, SROA GEP generation), I've come around to the idea that we should use i8 GEPs for opaque pointers, because the alternative would be to guess a GEP type from surrounding code, which will not be reliable. Ultimately, i8 GEPs is where we want to end up anyway, and opaque pointers just make that the natural choice.

(There are a couple of other places in SCEVExpander that check pointer element types, I plan to update those when I run across usable test coverage that doesn't assert elsewhere.)

Diff Detail

Event Timeline

nikic created this revision.Jul 3 2021, 11:52 AM
nikic requested review of this revision.Jul 3 2021, 11:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 3 2021, 11:52 AM
dblaikie added inline comments.
llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
642–643

can/should we avoid the use of PointerType::getElementType here by some means - preserving it through the various choices in the code above?

nikic added inline comments.Jul 6 2021, 12:58 PM
llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
642–643

This code path is only used for non-opaque pointers, so it's "okay" to use getElementType here. And I don't think we can avoid it -- if we know the element type to use, we wouldn't have to use an i8 GEP for opaque pointers either :)

dblaikie accepted this revision.Jul 6 2021, 1:14 PM

Sounds reasonable

llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
642–643

Ah, fair enough -and getElementType asserts on opaque pointers to check for this.

This revision is now accepted and ready to land.Jul 6 2021, 1:14 PM
This revision was landed with ongoing or failed builds.Jul 7 2021, 11:50 AM
This revision was automatically updated to reflect the committed changes.