This is an archive of the discontinued LLVM Phabricator instance.

[Attributor] Implement "noalias" callsite argument deduction
ClosedPublic

Authored by uenoku on Sep 6 2019, 9:13 AM.

Details

Summary

Now, nocapture is deduced in Attributor therefore, this patch introduces deduction for noalias callsite argument using nocapture.

Diff Detail

Repository
rL LLVM

Event Timeline

uenoku created this revision.Sep 6 2019, 9:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 6 2019, 9:13 AM
uenoku added inline comments.Sep 6 2019, 9:18 AM
llvm/lib/Transforms/IPO/Attributor.cpp
1675 ↗(On Diff #219124)

Once I thought it is not sound to do this initialization but, I reconsider that unless we don't manifest the information for floating value, there is no problem.

jdoerfert added inline comments.Sep 6 2019, 9:52 AM
llvm/test/Transforms/FunctionAttrs/noalias_returned.ll
235 ↗(On Diff #219124)

We need to make sure this test works as well:

define void @test12_3(){
  %A = tail call noalias i8* @malloc(i64 4)
  tail call void @two_args(i8* %A, i8* %A)
  ret void
}
uenoku added inline comments.Sep 6 2019, 11:08 AM
llvm/test/Transforms/FunctionAttrs/noalias_returned.ll
235 ↗(On Diff #219124)

If @two_args is:

declare void @two_args(i8* nocapture , i8* nocapture)

then

define void @test12_3(){
  %A = tail call noalias i8* @malloc(i64 4)
  tail call void @two_args(i8* noalias nocapture %A, i8* noalias nocapture%A)
  ret void
}

(If either argument is not marked as nocapture, there is no problem.)
It is not good ... I'll fix. We need to compare the call site argument and ensure that there is no copy.

jdoerfert added inline comments.Sep 6 2019, 11:32 AM
llvm/test/Transforms/FunctionAttrs/noalias_returned.ll
235 ↗(On Diff #219124)

We need to compare the call site argument and ensure that there is no copy.

Exactly. They need to be checked for aliases wrt. other arguments.

uenoku updated this revision to Diff 219244.Sep 7 2019, 8:16 AM

Add test

Except the nocapture changes this looks good to me. Can we make sure these are needed before we commit this?

llvm/lib/Transforms/IPO/Attributor.cpp
1732 ↗(On Diff #219244)

AANoCapture will get a "isCapturedBefore" functionality at some point to solve this.

1736 ↗(On Diff #219244)

make it isAssumedNoCaptureMaybeReturned.

2644 ↗(On Diff #219244)

I do not understand these changes. Why do they help? Can we extract them into a different commit?

2914 ↗(On Diff #219244)

Again, why is this needed?

uenoku updated this revision to Diff 219306.Sep 9 2019, 2:44 AM

Remove nocapture related change

uenoku marked 2 inline comments as done.Sep 9 2019, 2:53 AM
uenoku added inline comments.
llvm/lib/Transforms/IPO/Attributor.cpp
2644 ↗(On Diff #219244)

I used an internal attribute to look at deduction but anyway I don't need anymore.
However, I think we need to check whether the value is an argument or not. (D67342)

2914 ↗(On Diff #219244)

I encountered a runtime exception in test/Transform/FunctionAttrs/misc.ll. Once I added this check as a solution but now I find that it works well without the check.

jdoerfert accepted this revision.Sep 9 2019, 2:52 PM

LGTM.

This revision is now accepted and ready to land.Sep 9 2019, 2:52 PM
This revision was automatically updated to reflect the committed changes.