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

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 11 13:47:05 PST 2022


dblaikie added a comment.

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

> In D117778#3305983 <https://reviews.llvm.org/D117778#3305983>, @nikic wrote:
>
>> Any further thoughts on this? Some options I see:
>>
>> 1. Add `-normalize-opaque-pointers` to existing RUN lines. This allows them to pass when opaque pointers are enabled, but doesn't test opaque pointers in a default configuration.
>> 2. Add both `-opaque-pointers=1` and `-opaque-pointers=0 -normalize-opaque-pointers` to RUN lines. This makes everyone test both typed and opaque pointers in a default configuration.
>> 3. Add both `-opaque-pointers=1` and `-opaque-pointers=0` to RUN lines with different FileCheck prefixes.
>> 4. Add `-opaque-pointers` to RUN lines and stop testing typed pointers.
>> 5. ???
>>
>> I think all of these are principally viable, with different tradeoffs. This patch currently proposed version 1.
>
> 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.

Seems like a 5th option might be a buildbot that builds with opaque pointers on by default & a list of known failures suppressed? But maybe not worth the hassle to setup, not sure.


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

https://reviews.llvm.org/D117778



More information about the llvm-commits mailing list