This is an archive of the discontinued LLVM Phabricator instance.

[llvm] Update WPD tests to use opaque pointers
ClosedPublic

Authored by leonardchan on Oct 10 2022, 12:04 PM.

Diff Detail

Event Timeline

leonardchan created this revision.Oct 10 2022, 12:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 10 2022, 12:04 PM
leonardchan requested review of this revision.Oct 10 2022, 12:04 PM

Thanks. Per https://llvm.org/docs/OpaquePointers.html#version-support it sounds like non-opaque pointers are now only best effort and no longer need to be tested, since LLVM 15 has branched - is that correct?

llvm/test/Transforms/WholeProgramDevirt/import-no-dominating-assume.ll
0

"instrumented in with WPD" doesn't sound right to me. Maybe "added by WPD"?

@pcc added this test, hopefully he can advise. If we no longer need to test non-opaque pointers then it seems like this test could be removed, but hopefully he can confirm.

pcc added inline comments.Oct 10 2022, 1:25 PM
llvm/test/Transforms/WholeProgramDevirt/import-no-dominating-assume.ll
0

Yes, this test was added to test for an edge case that no longer exists with opaque pointers, so we should be able to remove it if we no longer test non-opaque pointers.

leonardchan marked 2 inline comments as done.

Thanks. Per https://llvm.org/docs/OpaquePointers.html#version-support it sounds like non-opaque pointers are now only best effort and no longer need to be tested, since LLVM 15 has branched - is that correct?

Waiting for confirmation on this. On that page, it also mentions that typed pointers will not be supported for llvm 16, so I'm not entirely sure if tests will need to be migrated as well. Ideally, I'd like to ensure any tests I add for relative vtables + [thin]lto match the existing tests for regular vtables. If these tests can stay the same with typed pointers, then I'll need to ensure some syntax involving dso_local_equivalent work with typed pointers to ensure test parity. Otherwise, I can ignore changes like D135483 and only support opaque pointers.

nikic added a comment.Oct 11 2022, 2:10 PM

Thanks. Per https://llvm.org/docs/OpaquePointers.html#version-support it sounds like non-opaque pointers are now only best effort and no longer need to be tested, since LLVM 15 has branched - is that correct?

Yes, this is correct.

tejohnson accepted this revision.Oct 11 2022, 3:24 PM

lgtm but just remove import-no-dominating-assume.ll

This revision is now accepted and ready to land.Oct 11 2022, 3:24 PM
This revision was automatically updated to reflect the committed changes.