[PATCH] D117778: [OpaquePtrs] Add -normalize-opaque-pointers option

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D117778#3260513 <https://reviews.llvm.org/D117778#3260513>, @nikic wrote:

> In D117778#3260225 <https://reviews.llvm.org/D117778#3260225>, @dblaikie wrote:
>
>> I think from that thread I still tend towards "introduce a build mode where it reads into opaque pointers (essentially auto-upgrade),
>
> Flipping the `opaque-pointers` option to true is effectively that -- we should probably make that default controlled by a cmake option.

Yeah, if that essentially enables the shipped version of opaque pointers (minus later API cleanup) - yeah, having a CMake option for that sounds awesome. Then people can try turning it on early, have a way to opt-out at least for a little while, let us run buildbots in the new mode before it ships, and continue testing in the old mode after it ships (briefly - to keep things passing in case we need to rollback/some users need a bit longer to migrate).

>> either have a list of known-failing tests in this mode, or some other way to flag on a per-test basis (maybe start with a "passing" list, and eventually move to a "failing" list when that's smaller)" - but I don't feel especially strongly/open to others perspectives.
>
> Looking at just Transforms tests, we have about 7k tests, of which 3k fail under opaque pointers, of which 0.5k crash. My primary concern here is that if we don't introduce something like this, we'll need to update 3k tests (in Transforms alone) during the opaque pointers switch. And we'll need to separately maintain those opaque-ified tests prior to the switch.
>
> I think the more changes we can make prior to the atomic "flip the flag" commit, the better.

Oh, for sure/agreed on this last point.

If we had the CMake flag described, and a buildbot for that config - we would update the CHECKs in these 3k tests to pass with/without opaque pointers, yeah? Ah, with the `-normalize-opaque-pointers` flag we wouldn't have to update these files to have agnostic (accounting for both typed and opaque pointers) CHECKs, they'd only have opaque pointers CHECKs - which is easier to do and means these don't eventually need to be cleaned up (or be weird/legacy phrasing) after the opaque pointers change is shipped?

Yeah, mixed feelings - happy to hear how other folks feel about what the tradeoffs are between these choices.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117778/new/

https://reviews.llvm.org/D117778