This is an archive of the discontinued LLVM Phabricator instance.

[JIT/llvm-rtdyld] Don't waste cycles invalidating the Instruction cache
ClosedPublic

Authored by davide on Oct 11 2015, 12:24 AM.

Details

Summary

We call InvalidateInstructionCache() twice here and I think the second call is just wasted work. Hopefully my guess is correct.

Diff Detail

Event Timeline

davide updated this revision to Diff 37048.Oct 11 2015, 12:24 AM
davide retitled this revision from to [JIT/llvm-rtdyld] Don't waste cycles invalidating the Instruction cache.
davide updated this object.
davide added a reviewer: lhames.
davide set the repository for this revision to rL LLVM.
davide added a subscriber: llvm-commits.
lhames added inline comments.Oct 13 2015, 11:06 PM
lib/Support/Unix/Memory.inc
266–282

Whatever we do here should be consistent for all platforms. I haven't taken a good look at the icache invalidation code - I'll have a think about this tomorrow.

Lang, what are your thoughts about this?
Should we invalidate the cache unconditionally instead?

Thanks!

davide updated this revision to Diff 38832.Oct 30 2015, 2:28 PM
davide removed rL LLVM as the repository for this revision.

Updated after our discussion on IRC. Thanks!

lhames edited edge metadata.Nov 16 2015, 8:39 AM

Hi Davide,

I spoke with Jim Grosbach about this and we agreed that 'setExecutable' should do everything required to make the memory executable on the host processor. That means unconditionally setting the permissions and invalidating the icache (on processors that need that). The problem is with clients who call InvalidateInstructionCache after calling setExecutable: that should be redundant. Clients should either call setExecutable alone, or call both protectMappedMemory and InvalidateInstructionCache manually.

So - we should remove the redundant calls, and modify setExecutable where necessary to properly set the permissions.

This comment was removed by lhames.
lhames accepted this revision.Nov 16 2015, 9:18 AM
lhames edited edge metadata.

LGTM, pending the removal of the icache invalidation for the data sections.

tools/llvm-rtdyld/llvm-rtdyld.cpp
393

The instruction cache doesn't need to be invalidated for data sections, so this loop should be removed.

This revision is now accepted and ready to land.Nov 16 2015, 9:18 AM
This revision was automatically updated to reflect the committed changes.