Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

change contents of ScalarEvolution from private to protected #83052

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

change contents of ScalarEvolution from private to protected #83052

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
eea887c
mainly pushing to switch machines
skewballfox Feb 25, 2024
e47436b
added AssumeLoopExits bool to SE, lifting MustExit code into SE
skewballfox Feb 29, 2024
f55e361
added MustExitcode for computeExitLimit
skewballfox Feb 29, 2024
8e85c06
added enzyme mustExit code to computeExitLimitFromSingleExitSwitch
skewballfox Feb 29, 2024
3f378b5
add enzyme must exit code to computeExitLimitFromCondImpl
skewballfox Feb 29, 2024
14a0c6c
implemented enzyme must exit code in computeExitLimitFromICmp
skewballfox Feb 29, 2024
abb0ab4
add Enzyme changes to SE howManyLessThans
skewballfox Feb 29, 2024
c1d83de
fixed issue in howManyLessThans where conditions were incorrectly dep…
skewballfox Mar 1, 2024
66ab0c3
incorporating changes from code review
skewballfox Mar 7, 2024
9b57191
removed unrelated change
skewballfox Mar 11, 2024
5776793
moved mustexit code to other computeExitLimitFromICmp definition
skewballfox Mar 14, 2024
bdabce8
reran git clang-format HEAD~1
skewballfox Mar 14, 2024
a9c9251
removed redundant binOp code from CondImpl
skewballfox Mar 14, 2024
7597a83
implenting requested changes
skewballfox Apr 20, 2024
16d19f6
forgot to remove include for deleted file
skewballfox Apr 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions llvm/include/llvm/Analysis/ScalarEvolution.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,10 @@ class ScalarEvolution {
LoopComputable ///< The SCEV varies predictably with the loop.
};

bool AssumeLoopExits = false;
skewballfox marked this conversation as resolved.
Show resolved Hide resolved
void setAssumeLoopExits();
llvm::SmallPtrSet<llvm::BasicBlock *, 4> GuaranteedUnreachable;
skewballfox marked this conversation as resolved.
Show resolved Hide resolved

/// An enum describing the relationship between a SCEV and a basic block.
enum BlockDisposition {
DoesNotDominateBlock, ///< The SCEV does not dominate the block.
Expand Down
70 changes: 70 additions & 0 deletions llvm/include/llvm/Analysis/Utils/EnzymeFunctionUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@

#include "llvm/Analysis/AliasAnalysis.h"
skewballfox marked this conversation as resolved.
Show resolved Hide resolved
#include "llvm/Analysis/LoopAnalysisManager.h"
#include "llvm/Analysis/TargetLibraryInfo.h"

#include "llvm/IR/Function.h"

#include "llvm/IR/Instructions.h"
#include "llvm/Transforms/Utils/ValueMapper.h"
#include <deque>

// TODO note this doesn't go through [loop, unreachable], and we could get more
// performance by doing this can consider doing some domtree magic potentially
static inline llvm::SmallPtrSet<llvm::BasicBlock *, 4>
getGuaranteedUnreachable(llvm::Function *F) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wsmoses Could you please provide some context on what's up with this whole "guaranteed unreachable" logic?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @wsmoses

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah so basically the gist is as follows.

Suppose we have some code from the loop that results in unreacahble blocks. For example:

for (int i=0; i<N; i++) {
    if (i > array.size()) {
       printf("Bounds error\n");
       exit(1);
    }
    something(i);
}

Again in our (enzyme) case here, we can assume we only need the loop limits if we successfully run the function (e.g. exit via a return). Thus we'd prefer to learn that this look has a bound of N, rather than unknown (since otherwise the loop could exit at an arbitrary time, via an exit call / unreachable).

This actually makes a huge performance difference in practice since a lot of codes have this bounds checking (and other related things), and not deducing the fixed bound forces us to use a dynamic reallocation rather than a static allocation size of N.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The guaranteed unreachable stuff is basically finding blocks/paths which are guaranteed to result in unreachable. E.g. if you had something like

if (Condition) {
  a()
} else {
 b()
}
exit(); // aka unreachable

we can ignore this entire sub-cfg even if the individual blocks terminators are not unreachable (since any path through the block will necessarily result in an unreachable).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the context of SE, I think the best way to integrate this may be a list of blocks/branches/etc for SE to ignore in the context of computing the available exits.

In this context we're choosing ones that are guaranteed to hit unreachable, but I think the nicer one to integrate would be something along the lines of IgnoredExitingBlocks. Of course to properly test this we may need to still have a version of getUnreachable somewhere to build a version of the SE exiting for loop test, but that way SE remains general

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://godbolt.org/z/KTY7sErqz

llvm::SmallPtrSet<llvm::BasicBlock *, 4> knownUnreachables;
if (F->empty())
return knownUnreachables;
std::deque<llvm::BasicBlock *> todo;
for (auto &BB : *F) {
todo.push_back(&BB);
}

while (!todo.empty()) {
llvm::BasicBlock *next = todo.front();
todo.pop_front();

if (knownUnreachables.find(next) != knownUnreachables.end())
continue;

if (llvm::isa<llvm::ReturnInst>(next->getTerminator()))
continue;

if (llvm::isa<llvm::UnreachableInst>(next->getTerminator())) {
knownUnreachables.insert(next);
for (llvm::BasicBlock *Pred : predecessors(next)) {
todo.push_back(Pred);
}
continue;
}

// Assume resumes don't happen
// TODO consider EH
if (llvm::isa<llvm::ResumeInst>(next->getTerminator())) {
knownUnreachables.insert(next);
for (llvm::BasicBlock *Pred : predecessors(next)) {
todo.push_back(Pred);
}
continue;
}

bool unreachable = true;
for (llvm::BasicBlock *Succ : llvm::successors(next)) {
if (knownUnreachables.find(Succ) == knownUnreachables.end()) {
unreachable = false;
break;
}
}

if (!unreachable)
continue;
knownUnreachables.insert(next);
for (llvm::BasicBlock *Pred : llvm::predecessors(next)) {
todo.push_back(Pred);
}
continue;
}

return knownUnreachables;
}