This is an archive of the discontinued LLVM Phabricator instance.

Basis of dropping uses in llvm.assume.
ClosedPublic

Authored by Tyker on Jan 24 2020, 11:48 PM.

Details

Summary

This patch adds the basic utilities to deal with dropable uses. dropable uses are uses that we rather drop than prevent transformations, for now they are limited to uses in llvm.assume.

Diff Detail

Event Timeline

Tyker created this revision.Jan 24 2020, 11:48 PM

We need to split this patch. The droppable logic can be applied to regular assumes already so we can separate this from the operand bundle stuff at first. That should make the patches significantly smaller. You also need to separate D73404 out.

llvm/lib/IR/Value.cpp
185

We have to make sure we handle "incomplete" operand bundles properly. I guess if we always go through KnowledgeRetention we can make sure of that.

Tyker updated this revision to Diff 241882.EditedFeb 1 2020, 9:03 AM
Tyker edited the summary of this revision. (Show Details)

i splited the patch.

Tyker updated this revision to Diff 241911.Feb 1 2020, 11:56 PM
Tyker retitled this revision from [WIP] Basis of droping uses in llvm.assume. to [WIP] Basis of dropping uses in llvm.assume..
Tyker retitled this revision from [WIP] Basis of dropping uses in llvm.assume. to Basis of dropping uses in llvm.assume..Feb 2 2020, 11:13 AM

I'm not sure i'm fully onboard with using generic 'droppable' term for these uses.
Can we be somehow more specific, since those are really only ever about assumes?

llvm/include/llvm/ADT/STLExtras.h
1534 ↗(On Diff #241911)

These changes can be separated into another review

Tyker added a comment.Feb 21 2020, 7:47 AM

I'm not sure i'm fully onboard with using generic 'droppable' term for these uses.
Can we be somehow more specific, since those are really only ever about assumes?

i don't know if it can be useful in other situations than for uses in assume.
if you think it probably won't we can just call then use in assume.

Tyker updated this revision to Diff 245860.Feb 21 2020, 8:05 AM

splitted the patch as requested.

There might be other operand bundle and metadata uses we want to drop later. Unclear. We could also rename it to "drop assume uses"

uenoku added a subscriber: uenoku.Feb 21 2020, 10:08 PM

I think this makes sense, especially since lifetime markers will soon join assume as droppable users. That said, we need to improve the nullptr replacement for assume. I made some suggestions here https://reviews.llvm.org/D73832#1902199

llvm/include/llvm/IR/User.h
224

We should list what those uses are, later maybe define an enum to restrict the selection.

Tyker updated this revision to Diff 248204.Mar 4 2020, 9:05 AM

Changes:

  • Dropped uses in assume are represented the tag ignore and the values are replaced with undef.
  • this broke the ordering invariance that hasAttributeInAssume was depending upon so i changed the implementation to a more naive linear search.
  • changed the test in KnowledgeRetentionTest.cpp to make them easier to write.
  • adapted the LangRef and verifier to the new "ignore" tag.

an assume containg a dropped use now looks like

call void @llvm.assume(i1 true) [ "ignore"(i32* undef, i64 8) ]
Tyker updated this revision to Diff 248999.Mar 8 2020, 9:08 AM

rebased

Yes, i suppose given that we now have a second user (lifetime), droppable isn't the worst choice.

jdoerfert accepted this revision.Mar 10 2020, 2:17 PM
jdoerfert added inline comments.
llvm/lib/IR/KnowledgeRetention.cpp
220

Can we give it a more descriptive name than Loop.

This revision is now accepted and ready to land.Mar 10 2020, 2:17 PM

LGTM. One comment though (see Loop)

Tyker added a comment.Mar 11 2020, 3:23 PM

This patch depends on D74967 which is not yet accepted.

This revision was automatically updated to reflect the committed changes.