When attempting to split a coroutine with 'hidden' visibility (for
example, a C++ coroutine that is inlined when compiled with the option
'-fvisibility-inlines-hidden'), LLVM would hit an assertion in
include/llvm/IR/GlobalValue.h:240: "local linkage requires default
visibility". The issue is that the visibility is copied from the source
of the function split in the CloneFunctionInto function, but the linkage
is not. To fix, create the new function first with external linkage,
then copy the linkage from the original function *after* CloneFunctionInto
is called.
Since GlobalValue::setLinkage in turn calls maybeSetDsoLocal, the
explicit call to setDSOLocal can be removed in CoroSplit.cpp.
Test Plan: check-llvm
@rnk This function, CloneFunctionInto, calls Function::copyAttributesFrom, which in turn calls GlobalValue::copyAttributesFrom and finally GlobalValue::setVisibility, which triggers the assert when the original function F has hidden linkage.
That sounds like a good idea to me, too. It might be a little tricky because the copying occurs several layers deep.
Perhaps I could add a Function::copyAttributesFrom overload that also takes, as an argument, a list of attributes to not copy? I could then prevent the visibility attribute from being copied here, in lib/Transforms/Utils/CloneFunction.cpp:
void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc, ValueToValueMapTy &VMap, bool ModuleLevelChanges, SmallVectorImpl<ReturnInst*> &Returns, const char *NameSuffix, ClonedCodeInfo *CodeInfo, ValueMapTypeRemapper *TypeMapper, ValueMaterializer *Materializer) { // ... // Copy all attributes other than those stored in the AttributeList. We need // to remap the parameter indices of the AttributeList. AttributeList NewAttrs = NewFunc->getAttributes(); NewFunc->copyAttributesFrom(OldFunc); NewFunc->setAttributes(NewAttrs); // ... }It seems like it's the intention of this code to only copy attributes that aren't set on the new function, so an overload may express this intent even better. And preventing the visibility from being copied should prevent this assert from being hit as well. What do you think? Does that sound like a reasonable approach?