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

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D117778#3306336 <https://reviews.llvm.org/D117778#3306336>, @aeubanks wrote:

> My assumption is that most things work with opaque pointers and we'll fix and add tests for the few things that specifically are broken by opaque pointers.
> I think 1) is fine, although I still sorta prefer 4) but waiting until we've completely flushed out all opaque pointer issues mostly because I don't really want to burden people not working on opaque pointers with another thing to worry about (what is this `ptr` thing? why does the output IR look slightly different than the input IR? why do all these tests have `-normalize-opaque-pointers`?) during the opaque pointer fixup period which could last a while. It's possible that opaque pointer issues we aren't specifically testing could regress, but IMO that's unlikely and also easy to pinpoint and fix. But I could be convinced to do 1).
> Perhaps one issue is testing of typed pointers post-opaque pointers, which is where 1) would be useful. Not sure how much we want to commit to having rigorous testing of typed pointers when opaque pointers are the default for in tree projects.
> I don't like 2) and 3), they're too much overhead for not enough value.

Okay, my original thinking here was that we should have most of the llvm lit tests passing with opaque pointers before they get enabled in clang, but on further consideration that may not be particularly valuable. Basically, cases where opaque pointers are improperly handled will either crash outright (which we can easily check), or won't have existing test coverage anyway.

I guess the chicken and egg problem here gets solved by enabling opaque pointers in clang despite having limited direct test coverage in LLVM, and then converting LLVM tests after that has happened (and dropping typed pointer coverage while doing that). That is probably less risky than the other way around (first migrating the tests and then flipping what clang uses) by dint of typed pointers being harder to get right in terms of inserting all the necessary bitcasts.

In that case what I'm proposing here might actually be more useful on the clang side? It would allow us to convert all the clang tests to use opaque pointer output in advance, rather than doing a huge mass change as part of flipping the flag (which is likely going to be reverted a couple of times).

I do think we've reached the point where it's fine to mass-include `ptr` in tests and make other people deal with opaque pointers. Opaque pointers basically work for end-to-end optimized compiles of C/C++ now, and we're now working on long-tail issues now.

I plan to start a discourse thread soon to get some wider input on how to handle the opaque pointer switch (and make sure that people with out-of-tree code are aware that a switch will happen soon...)


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

https://reviews.llvm.org/D117778