avl added a comment.
> It's OK to set "tail" on any call that satisfies these requirements (from
> https://llvm.org/docs/LangRef.html#call-instruction): "Both markers [tail and
> musttail] imply that the callee does not access allocas from the caller. The
> tail marker additionally implies that the callee does not access varargs from
> the caller."
> "tail" does not mean that the call *must* be generated as a tail call. It
> just means that it's safe to generate it as a tail call if it turns out to be
> possible (e.g. if the compiler can prove that @noarg doesn't return, or if it
> can prove that all the code after the call to @noarg has no effect, or so on).
Yes, that is understood: ""tail" does not mean that the call *must* be
generated as a tail call".
I intended to make picture consistent: when markTails sees function body, it is
evident that the first call is not a tail call. I agree that a compiler "can
prove that @noarg doesn't return, or if it can prove that all the code after
the call to @noarg has no effect" and this first call could become tail-call.
Probably the suitable strategy, in this case, would be to recalculate marking
when it is broken(instead of creating false positive marks). There are other
cases(compiler could add function calls), which could break existing marking
and then would be necessary to recalculate marking.
Having many false positive "tail" marks could create a confusing picture.
I would describe the full problem which I am trying to solve.
(I assumed this patch would be the first one for that problem):
cat test.cpp
int count;
__attribute__((noinline)) void globalIncrement(const int* param) { count +=
*param; }
void test(int recurseCount)
{
if (recurseCount == 0) return;
{
int temp = 10;
globalIncrement(&temp);
}
test(recurseCount - 1);
}
TRE is not done for that test case. There are two reasons for that:
First is that AllocaDerivedValueTracker does not use the PointerMayBeCaptured
interface, and it does not see that &temp is not escaped.
Second is that AllCallsAreTailCalls is used as a pre-requisite for making TRE.
So it requires that all calls would be marked as "tail". This looks too
restricted.
Instead, it should check that "&temp" is not escaped in globalIncrement() and
that "test"
is a tail recursive call not using allocas. I think the confusion happened
exactly because "tail" marking was done for all calls(not for the real
tailcalls).
Thus I planned to do the following fixes:
1. cleanup "tail" marking.(this patch)
2. do not use "AllCallsAreTailCalls" as a pre-requisite for TRE.(this patch).
3. use PointerMayBeCaptured inside AllocaDerivedValueTracker.
What do you think about all of this?
It seems to me that it would be good to have consistent "tail" marking.
But if it does not look like a good idea, I could continue to point 2. and 3.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82085/new/
https://reviews.llvm.org/D82085
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
