In a thunk DSE deletes a store to the stack. This causes an incorrect
parameter value (for a byval argument) to be passed on to a tail call.
This exposes a corner case that requires clarification. I believe for a byval in
a tail call, the pass by value semantics of byval holds but not the ‘owned by
caller’ aspect of it. That part does not make sense for tail call. And this
justifies the code in getModRefInfo. DSE looks at the parameter list of the
call, but asks whether or not the call could mod ref a stack location. Since
this is a tail call I think mod ref analysis correctly asserts “ no mod ref “.
Here is the relevant code in BasicAliasAnalysis.cpp/getModRefInfo():
If this is a tail call and Loc.Ptr points to a stack location, we know that
the tail call cannot access or modify the local stack.
We cannot exclude byval arguments here; these belong to the caller of
the current function not to the current function, and a tail callee
// may reference them.
if (const CallInst *CI = dyn_cast<CallInst>(CS.getInstruction())) if (CI->isTailCall()) return MRI_NoModRef;
The information DSE misses out on is whether the call arguments need values from
the runtime stack. The patch is adding that check. Does anyone have different
opinion about the fix?
Also, FWIW, I think a tail call with a byval argument can happen only in a
thunk. That assertion is missing yet.