This is an archive of the discontinued LLVM Phabricator instance.

[Attributor] Add AAValueSimplifyCallSiteArgument::manifest
ClosedPublic

Authored by okura on Jun 30 2020, 4:05 AM.

Details

Summary

Value simplification result in AAValueSimplify is sometimes flow-sensitive because of AAConstantRange::getConstantRangeFromLVI,AAConstantRange::getConstantRangeFromSCEV.
But manifestation for callsite argument is done in the same way for the floating position. This causes the following unexpected simplification, for example.
Before:

define i32 @g(i1 %d) {
  br i1 %c, label %t, label %f
t:
  %ret1 = call i32 @f(i1 %c)
  ret i32 %ret1
f:
  %ret2 = call i32 @f(i1 false)
  ret i32 %ret2
}

After:

define i32 @g(i1 %d) {
  br label %t
t:
  %ret1 = call i32 @f(i1 true)
  ret i32 %ret1
f:
  %ret2 = call i32 @f(i1 false)
  ret i32 %ret2
}

This patch fix the problem by replacing use with simplified values in manifestation.

Diff Detail

Event Timeline

okura created this revision.Jun 30 2020, 4:05 AM
Herald added a reviewer: uenoku. · View Herald Transcript
Herald added a reviewer: homerdin. · View Herald Transcript
Herald added a reviewer: baziotis. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
jdoerfert accepted this revision.Jun 30 2020, 5:50 AM

LGTM. One nit

llvm/lib/Transforms/IPO/AttributorAttributes.cpp
4686

Move the U into the conditional or make if (!C) return an early exit.

This revision is now accepted and ready to land.Jun 30 2020, 5:50 AM
okura updated this revision to Diff 274797.Jul 1 2020, 7:18 AM

corrected the point.

Can you merge this?

okura updated this revision to Diff 277281.Jul 12 2020, 2:47 AM

fix test

okura added a comment.Jul 12 2020, 3:39 AM

Can you merge this?

Do I have a right to merge this by myself? I did arc patch and tried to git push https://github.com/llvm/llvm-project.git HEAD:master according to the document, but I failed to do that.

sstefan1 added a comment.EditedJul 12 2020, 4:06 AM

Can you merge this?

Do I have a right to merge this by myself? I did arc patch and tried to git push https://github.com/llvm/llvm-project.git HEAD:master according to the document, but I failed to do that.

Did you get the commit access? If so, what problems did you have with git push?

okura added a comment.Jul 12 2020, 4:53 AM

Can you merge this?

Do I have a right to merge this by myself? I did arc patch and tried to git push https://github.com/llvm/llvm-project.git HEAD:master according to the document, but I failed to do that.

Did you get the commit access? If so, what problems did you have with git push?

I got the following message with git push

remote: Permission to llvm/llvm-project.git denied to okuraofvegetable.
fatal: unable to access 'https://github.com/llvm/llvm-project.git/': The requested URL returned error: 403

From this message, it seems to me that I don't have commit access.

kuter added a comment.EditedJul 12 2020, 4:58 AM

Can you merge this?

Do I have a right to merge this by myself? I did arc patch and tried to git push https://github.com/llvm/llvm-project.git HEAD:master according to the document, but I failed to do that.

Did you get the commit access? If so, what problems did you have with git push?

I got the following message with git push

remote: Permission to llvm/llvm-project.git denied to okuraofvegetable.
fatal: unable to access 'https://github.com/llvm/llvm-project.git/': The requested URL returned error: 403

From this message, it seems to me that I don't have commit access.

You are not a member of the llvm org in github.
If you asked for commit access you should have received a invite.

If you where it should show up in your github profile
https://github.com/okuraofvegetable

bbn added a comment.Jul 12 2020, 5:00 AM

Take a look at this page: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access , you need to send an email to Chris to ask for commit access.

okura added a comment.Jul 12 2020, 5:22 AM

Thank you, everyone. I sent a request e-mail.

This revision was automatically updated to reflect the committed changes.