This is an archive of the discontinued LLVM Phabricator instance.

[ms] Emit vtordisp initializers in a deterministic order.
ClosedPublic

Authored by thakis on Mar 7 2018, 12:11 PM.

Details

Reviewers
rnk
Summary

No effective behavior change, just for cleanliness.

Fixes PR36159.

Diff Detail

Event Timeline

thakis created this revision.Mar 7 2018, 12:11 PM
thakis added inline comments.Mar 7 2018, 12:22 PM
lib/CodeGen/MicrosoftCXXABI.cpp
1207

(I added the missing & here locally.)

efriedma added inline comments.
lib/CodeGen/MicrosoftCXXABI.cpp
1202

Using stable_sort() here, as opposed to sort(), doesn't do anything useful here; VBaseMap is a DenseMap with a pointer key, so the input is in random order anyway.

thakis updated this revision to Diff 137463.Mar 7 2018, 1:14 PM

sort unstably

thakis marked an inline comment as done.Mar 7 2018, 1:14 PM
rnk added a comment.Mar 7 2018, 1:20 PM

Thanks for investigating!

lib/CodeGen/MicrosoftCXXABI.cpp
1206

I think we can avoid the sort altogether if we iterate RD->vbases(), which should already be in layout order, and then do lookup into VBaseMap.

thakis updated this revision to Diff 137470.Mar 7 2018, 1:50 PM

rnk comment

thakis marked an inline comment as done.Mar 7 2018, 1:50 PM
thakis added inline comments.
lib/CodeGen/MicrosoftCXXABI.cpp
1206

That's way nicer, thanks. Done.

rnk accepted this revision.Mar 7 2018, 1:57 PM

lgtm

lib/CodeGen/MicrosoftCXXABI.cpp
1207

The getVBaseClassOffset call turns around and does the same hashtable lookup we just did. Let's keep the iterator around and say I->second.VBaseOffset.getQuantity().

This revision is now accepted and ready to land.Mar 7 2018, 1:57 PM
thakis closed this revision.Mar 7 2018, 3:18 PM
thakis marked an inline comment as done.

r326960, thanks!