-
Notifications
You must be signed in to change notification settings - Fork 97
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.
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
Add support for tail calls #1090
base: master
Are you sure you want to change the base?
Add support for tail calls #1090
Conversation
There are a few tests (including this one) which are currently failing because the target function has undergone TailCallElim (as part of optimize_module), thus the refined calls get tail'd. Should we bail out from |
@@ -5,7 +5,7 @@ target triple = "x86_64-unknown-unknown" | |||
declare i32 @memcmp(ptr nocapture, ptr nocapture, i64) | |||
|
|||
define i32 @src(ptr nocapture readonly %x, ptr nocapture readonly %y) { | |||
%call = tail call i32 @memcmp(ptr %x, ptr %y, i64 2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert this change; it should be fine?
ret void | ||
} | ||
|
||
define void @tgt() { | ||
tail call void (ptr, ...) @g(ptr null) | ||
call void (ptr, ...) @g(ptr null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert
@@ -24,7 +24,7 @@ for.body: ; preds = %entry, %for.body | |||
|
|||
for.end: ; preds = %for.body | |||
%3 = load i32, ptr @b, align 4 | |||
%call = tail call i32 (ptr, ...) @print(ptr nonnull dereferenceable(1) getelementptr inbounds ([15 x i8], ptr @.str, i64 0, i64 0), i32 %3) | |||
%call = call i32 (ptr, ...) @print(ptr nonnull dereferenceable(1) getelementptr inbounds ([15 x i8], ptr @.str, i64 0, i64 0), i32 %3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert
@@ -74,7 +74,7 @@ for.end: ; preds = %for.end_us_lcssa, % | |||
store i32 1, ptr @d, align 4 | |||
store i32 %inc.lcssa, ptr @b, align 4 | |||
store i32 %conv.lcssa, ptr @f, align 4 | |||
%call = tail call i32 (ptr, ...) @myprintf(ptr getelementptr inbounds ([4 x i8], ptr @.str, i64 0, i64 0), i32 %conv.lcssa) | |||
%call = call i32 (ptr, ...) @myprintf(ptr getelementptr inbounds ([4 x i8], ptr @.str, i64 0, i64 0), i32 %conv.lcssa) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert
@@ -39,7 +39,7 @@ if.end: ; preds = %delete.end, %entry | |||
define i32 @tgt(ptr %bp) { | |||
entry: | |||
%add.ptr = getelementptr inbounds i32, ptr %bp, i64 1 | |||
tail call void @_ZdaPv(ptr %add.ptr) | |||
call void @_ZdaPv(ptr %add.ptr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert
public: | ||
Memcmp(Type &type, std::string &&name, Value &ptr1, Value &ptr2, Value &num, | ||
bool is_bcmp): MemInstr(type, std::move(name)), ptr1(&ptr1), | ||
ptr2(&ptr2), num(&num), is_bcmp(is_bcmp) {} | ||
bool is_bcmp, bool is_tailcall): MemInstr(type, std::move(name)), ptr1(&ptr1), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is over 80 chars
@@ -171,6 +171,9 @@ class State { | |||
// The value of a 'returned' input | |||
std::optional<StateValue> returned_input; | |||
|
|||
// Used to keep track of the current instruction | |||
const Value *current_value = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't seem needed. Instructions can pass *this to check_tailcall.
|
||
auto it = s.getFn().bbOf(*call).instrs().begin(); | ||
bool found = false; | ||
for (auto e = s.getFn().bbOf(*call).instrs().end(); it != e; ++it) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls don't call bbOf() twice. you can have a single loop to fetch the instruction after the call.
I think overall this is the right direction, thanks! |
Introduce support for tail calls: any subsequent access in the caller is UB after a tail call-site (bool only approach).