This is an archive of the discontinued LLVM Phabricator instance.

[APInt] Remove self move check from move assignment operator
ClosedPublic

Authored by craig.topper on Apr 16 2017, 1:21 AM.

Details

Summary

This was added to work around a bug in MSVC 2013's implementation of stable_sort. That bug has been fixed as of MSVC 2015 so we shouldn't need this anymore.

Technically the current implementation has undefined behavior because we only protect the deleting of the pVal array with the self move check. There is still a memcpy of that.VAL to VAL that isn't protected. In the case of self move those are the same local and memcpy is undefined for src and dst overlapping.

This reduces the size of the opt binary on my local x86-64 build by about 4k.

Diff Detail

Event Timeline

craig.topper created this revision.

Fix diff context

hans added inline comments.Apr 17 2017, 10:50 AM
include/llvm/ADT/APInt.h
695–696

Should we assert that this != &that ?

704

Actually, couldn't we just memcpy/memove both VAL and BitWidth, i.e. the whole APInt? Don't know if that makes any difference though.

705–706

This isn't strictly necessary right, just for finding bugs? Maybe we should only do it for asserts builds?

craig.topper added inline comments.Apr 17 2017, 11:05 AM
include/llvm/ADT/APInt.h
695–696

I'll add that.

705–706

Setting the bitwidth of 'that' to 0 is needed so 'that' doesn't try to delete the pVal array in its destructor.

craig.topper marked an inline comment as done.
hans accepted this revision.Apr 17 2017, 11:42 AM

Oh right, forgot the other one has a destructor :-)

lgtm

This revision is now accepted and ready to land.Apr 17 2017, 11:42 AM
This revision was automatically updated to reflect the committed changes.