Page MenuHomePhabricator

[OpaquePtr] Remove existing support for forward compatibility
ClosedPublic

Authored by aeubanks on Tue, May 25, 8:13 PM.

Details

Summary

It assumes that PointerType will keep having an optional pointee type,
but we'd like to remove the pointee type in PointerType at some point.

I feel like the current implementation could be simplified anyway,
although perhaps I'm underestimating the amount of work needed
throughout BitcodeReader.

We will still need a side table to keep track of pointee types. This
will be reimplemented at some point.

This is essentially a revert of a4771e9d (which doesn't look like it was
reviewed anyway).

Diff Detail

Event Timeline

aeubanks created this revision.Tue, May 25, 8:13 PM
aeubanks requested review of this revision.Tue, May 25, 8:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptTue, May 25, 8:13 PM

Yeah, can't say I fully follow either the original patch or the revert (sort of follow the motivation for the revert). Would love to get @t.p.northover's thoughts here.

aeubanks updated this revision to Diff 350717.Tue, Jun 8, 2:36 PM

remove one more function

@t.p.northover seems to not be responding, can we go forward with this?

The motivation is that this implementation is assuming that PointerType will always have a pointee type. See

/// Map all pointer types within \param Ty to the opaque pointer
/// type in the same address space if opaque pointers are being
/// used, otherwise nop. This converts a bitcode-reader internal
/// type into one suitable for use in a Value.
Type *flattenPointerTypes(Type *Ty) {
  return Ty;
}

But we want to remove the pointee type in PointerType at some point.

dblaikie accepted this revision.Fri, Jun 11, 1:10 PM

Fair enough

This revision is now accepted and ready to land.Fri, Jun 11, 1:10 PM
This revision was landed with ongoing or failed builds.Mon, Jun 14, 10:53 AM
This revision was automatically updated to reflect the committed changes.