[PATCH] D85614: [TRE] Reland: allow TRE for non-capturing calls.

[Alexey Lapshin via Phabricator via llvm-commits]

avl added a comment.

> Among all the attributes that can be attached to an argument of a tail call, byval is unusual. It significantly changes the semantics of the call: when the call executes, instead of passing the pointer itself, it makes a copy of the data behind the pointer.
>
> Consider, for example, the following testcase on x86-64:
>
> typedef struct A { long long x[100]; } A;
> A global;
> int printf(const char*, ...);
> void dostuff(A arg, int i) { if (i==100) return; arg.x[5]++; printf("%lld\n", arg.x[i]); dostuff(global, i+1); }
> __attribute((optnone)) int main() { dostuff(global, 0); }
>
> dostuff has a byval argument. At the point of the call, the compiled code is supposed to copy that argument onto the stack, so in the callee the stack contains the values of all the array elements. If this is working correctly, the fifth line of the output will be "1": each call copies the value from the global, so we only ever increment the array element once. (With optimizations enabled, clang currently gets this wrong.)

It looks like current patch works correctly with this test case.

> If we're doing tail-call elimination, in general, we need to make this implicit copy explicit. One way to implement this is to generate a pair of copies for each byval argument. First, create a temporary alloca corresponding to each byval argument. Then, copy each byval argument to the call to its temporary alloca. Finally, copy each byval argument to the call from the temporaries to the memory originally allocated for the function's arguments.
>
> The copies have to happen in the order I described in case a byval argument to the call refers to a different byval argument in the caller. For example, if you have something along the lines of void dostuff(A arg, Arg b) { dostuff(b, a); }

yep. this patch works incorrectly for "dostuff(A arg, Arg b) { dostuff(b, a); }" case. Thank you for pointing that!
It requires additional temp to store byval argument. will update the patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85614/new/

https://reviews.llvm.org/D85614