This is an archive of the discontinued LLVM Phabricator instance.

Update optimization passes to handle inalloca arguments
ClosedPublic

Authored by rnk on Dec 19 2013, 2:43 PM.

Details

Summary

I searched Transforms/ and Analysis/ for 'ByVal' and updated those call
sites to check for inalloca if appropriate.

I added tests for any change that would allow an optimization to fire on
inalloca.

Diff Detail

Event Timeline

nlewycky added inline comments.Jan 14 2014, 6:42 PM
include/llvm/IR/Argument.h
64

"the byval attribute or inalloca attribute"
or *the inalloca attribute?

"from the perspective of the callee" doesn't really tell the reader much (why are they similar?). Suggestion: "These attributes both represent arguments being passed by copy under different ABIs."

include/llvm/IR/Instructions.h
35

Sort.

115

Unclear what "static" means here.

120

Optional: Why do you need isUsedWithInAlloca() instead of just calling this and testing the result? CallSite evaluates to false when zero-initialized.

lib/IR/Instructions.cpp
935

No action required: By the way, this is actually O(n^2) not O(n) because attributes are stored in a linked list so finding 1+ArgNo is not constant time. See PR16536.

lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
84

"considered to be" --> "implicitly"? You get undef if you load from one afterwards, right?

rnk updated this revision to Unknown Object (????).Jan 17 2014, 5:21 PM

Rebase everything on top of redesigned inalloca

rnk added a comment.Jan 17 2014, 5:21 PM

A lot of this changed and I think the questionable parts went away. PTAL

include/llvm/IR/Argument.h
64

Done.

include/llvm/IR/Instructions.h
35

Gone.

115

It's unclear immediately after "isStaticAlloca()?"

120

Gone now, but I thought it read better.

lib/IR/Instructions.cpp
935

Interesting. This code is gone now. =/

lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
84

Or just "are clobbered..."

rnk updated this revision to Unknown Object (????).Jan 17 2014, 5:24 PM

Revert CallSite.h change

rnk updated this revision to Unknown Object (????).Jan 21 2014, 11:21 AM
  • tweak comments and add a test case.
nlewycky accepted this revision.Jan 27 2014, 6:29 PM

Please merge test/Transforms/ArgumentPromotion/inalloca.ll and test/Transforms/ArgumentPromotion/inalloca-2.ll

rnk added a comment.Jan 27 2014, 6:40 PM

Done. Thanks!

rnk closed this revision.Jan 27 2014, 6:44 PM

Closed by commit rL200281 (authored by @rnk).