aaron.ballman added inline comments.
================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1302
// in case the function pointer is referenced somewhere.
- if (FD->isInlineBuiltinDeclaration() && Fn) {
+ if (Fn) {
std::string FDInlineName = (Fn->getName() + ".inline").str();
----------------
nickdesaulniers wrote:
> I don't think we want to do all this work if just `Fn`; ie. create a new
> `std::string` with `.inline` suffix for every function we're going to
> generate code (IR) for. How about we add an additional unlikely guard:
>
> `if (FD->getBuiltinID() && FN) {`
>
> Because in the usual case, `FD` both has a builtin ID and is an inline
> builtin declaration, while in the exceptional case that this patch addresses,
> `FD` has a builtin ID but is not an inline builtin declaration.
Is it correct to gate this on whether it's a builtin or not? I thought that
builtin-like (e.g., the usual pile of attributes) user code should also have
the same effect, shouldn't it?
================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1319-1321
+ // everywhere. That's GCC behavior too. Unfortunately, I cannot find a way
+ // to detect that situation before we reach codegen, so do some late
+ // replacement.
----------------
nickdesaulniers wrote:
> Perhaps in `Sema::CheckFunctionDeclaration`? I see there is where we detect
> redeclarations. The calls from there to
> `FunctionDecl::setPreviousDeclaration()` seem to set up the redecl chain.
> Perhaps this exceptional case (or both cases, even) would be handled better
> there?
>
> cc @rsmith @aaron.ballman in case they have feedback/tips/cycles.
I don't know that it's a good idea to modify the redeclaration chain in this
case. The comments on the chain are pretty clear that it's a temporal chain
where "previous" means previously declared in relation to the current
declaration. @rsmith may feel differently, however.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112059/new/
https://reviews.llvm.org/D112059
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
