[PATCH] D156793: [TailCallElim] Remove the readonly attribute of byval.

Alexey Lapshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 2 08:03:14 PDT 2023

avl added inline comments.

Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:683
+      // attribute is removed.
+      F.removeParamAttr(I, Attribute::ReadOnly);
       ArgumentPHIs[I]->addIncoming(F.getArg(I), BB);
DianQK wrote:
> avl wrote:
> > probably, instead of removing readonly attribute we could stop generating it? Otherwise it will be presented or not depending of whether tail recursion elimination is happened.
> > 
> > ```
> > byval(<ty>)
> > 
> >     This indicates that the pointer parameter should really be passed by value to the function. The attribute implies that a hidden copy of the pointee is made between the caller and the callee, so the callee is unable to modify the value in the caller.
> > 
> > ```
> > 
> > The hidden copy protects original value from modifications done by callee. Having readonly attribute for hidden copy, probably, redundant.
> Do you mean don't add `readonly` at PostOrderFunctionAttrsPass?
> I don't think so. First rustc/clang can also add the `readonly` attribute.
> I think `readonly` + `byval` makes sense. There are more internal invariant representations than only `byval`.
if readonly is important then we probably should not do tail recursion elimination for readonly + byval as we will write into the readonly data. if it is not important then it should be safe to not set this flag?

  rG LLVM Github Monorepo



More information about the llvm-commits mailing list