This is an archive of the discontinued LLVM Phabricator instance.

Improve what can be promoted in LICM.
ClosedPublic

Authored by trentxintong on Jan 17 2017, 4:29 PM.

Details

Summary

In case of non-alloca pointers, we check for whether it is a pointer
from malloc-like calls and it is not captured. In such case, we can
promote the pointer, as the caller will have no way to access this pointer
even if there is unwinding in middle of the loop.

Diff Detail

Repository
rL LLVM

Event Timeline

trentxintong created this revision.Jan 17 2017, 4:29 PM
efriedma added inline comments.
lib/Transforms/Scalar/LICM.cpp
1014–1015 ↗(On Diff #84776)

Is there an existing testcase which covers this?

1018 ↗(On Diff #84776)

Can we avoid calling PointerMayBeCaptured twice? (We perform a similar check later in this function.)

test/Transforms/LICM/scalar-promote-unwind.ll
173 ↗(On Diff #84776)

It would be more useful to have a testcase which doesn't leak memory.

Address comments.

  1. Call PointerMayBeCaptured only once.
  2. 2 test cases for malloced memory, first can escape and the other cant.

Make sure only the latter is promoted.

Cleanup LIT test file.

We do not check for MayBeCaptured for alloca when there is throw in the loop.

efriedma added inline comments.Jan 18 2017, 1:29 PM
lib/Transforms/Scalar/LICM.cpp
1026 ↗(On Diff #84799)

Maybe reorganize this so you don't call isAllocLikeFn twice?

1136 ↗(On Diff #84799)

I don't think you need to check isAllocLikeFn here.

Address comments.

efriedma added inline comments.Jan 18 2017, 2:47 PM
lib/Transforms/Scalar/LICM.cpp
1028 ↗(On Diff #84888)

I was thinking of something more like if (!isa<AllocaInst>()) { if (!isAllocLikeFn()) return false; [...] }.

test/Transforms/LICM/scalar-promote-unwind.ll
211 ↗(On Diff #84888)

This doesn't really test what you want it to... it would fail the aliasing test anyway.

Actually, hmm, it's hard to come up with a practical test where the PointerMayBeCaptured check is actually necessary: you need a call which could throw, but provably doesn't access a malloc whose address is captured... our current AA infrastructure can't prove something like that given the way C++ unwinding works. I guess you could construct an artificial IR testcase with a call that's readnone but not nothrow.

trentxintong added inline comments.Jan 18 2017, 3:35 PM
test/Transforms/LICM/scalar-promote-unwind.ll
211 ↗(On Diff #84888)

yea, thats what i can think off right now. the problem is if the ptr escapes, then the function that actually throws will potentially be able to read/write it ...

Address comments.

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

@efriedma Thanks for the review !