Page MenuHomePhabricator

[GlobalsAA] Restrict ModRef result if any internal method has its address taken.
ClosedPublic

Authored by asbirlea on Oct 31 2019, 4:35 PM.

Details

Summary

If there are any internal methods whose address was taken, conclude there is nothing known in relation of any other internal method and a global.

Diff Detail

Event Timeline

asbirlea created this revision.Oct 31 2019, 4:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 31 2019, 4:35 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
asbirlea updated this revision to Diff 227508.Nov 1 2019, 12:45 PM

Set KnowNothing.

asbirlea retitled this revision from [GlobalsAA] Set MayReadAnyGlobal when nothing is known about a function. to [GlobalsAA] Set KnowNothing when nothing is known about a function..Nov 1 2019, 12:46 PM
asbirlea edited the summary of this revision. (Show Details)
nlopes accepted this revision.Nov 2 2019, 12:47 PM

LGTM!

This revision is now accepted and ready to land.Nov 2 2019, 12:47 PM

I think this version of the test case is slightly oversimplified and it would be valid for an optimisation to remove the check. If there is any address-taken function that may have escaped, then it is not okay to assume that @llvm.objc.autoreleasePoolPop will definitely not write to it, but in this case there are no stores to an internal global and so GVN is at liberty to eliminate it entirely. The fact that it doesn't is a missed optimisation caused by not having sufficient knowledge of @llvm.objc.autoreleasePoolPop.

In the original version of the test case, the miscompile comes from the fact that a -dealloc method stores to deallocCalled. The -dealloc method is registered with the runtime and is in a data structure that visible to the runtime.

asbirlea updated this revision to Diff 227943.Nov 5 2019, 11:58 AM

I updated the patch to be more generic, while still addressing this usecase.
The condition is: if there are any internal methods whose address is taken but for which we do not conclude that they do not modify globals, then cannot conclude lower than ModRef for any other (LocalLinkageCall, GlobalValue).
Tests included.

asbirlea updated this revision to Diff 227945.Nov 5 2019, 12:09 PM

Remove missed comment.

asbirlea retitled this revision from [GlobalsAA] Set KnowNothing when nothing is known about a function. to [GlobalsAA] Restrict ModRef result if any internal method has its address taken..Nov 5 2019, 12:12 PM
asbirlea edited the summary of this revision. (Show Details)
This comment was removed by theraven.
This revision was automatically updated to reflect the committed changes.

Do I understand correctly that this going to be conservative in the case the internal function with its address taken is readonly/none?
Either way, could we have a test for that to make sure we see the "good" behavior or a FIXME in the code and test explaining what is needed?