nickdesaulniers added a comment. Bumping for an update here. We can tolerate a build breakage for our older kernels over the weekend, but we should really try to get this resolved by EOW, otherwise we need to look into reverting:
- 3d6f49a56995b845c40be5827ded5d1e3f692cec <https://reviews.llvm.org/rG3d6f49a56995b845c40be5827ded5d1e3f692cec> Tue Sep 28 13:24:25 2021 +0200 (breakage) - bd379915de38a9af3d65e19075a6a64ebbb8d6db <https://reviews.llvm.org/rGbd379915de38a9af3d65e19075a6a64ebbb8d6db> Tue Sep 28 16:07:33 2021 +0200 (attempted fix forward of 3d6f49a56995b845 <https://reviews.llvm.org/rG3d6f49a56995b845c40be5827ded5d1e3f692cec>) - 0d76d4833dd2815e0b1c786250f474d222f6a0a1 <https://reviews.llvm.org/rG0d76d4833dd2815e0b1c786250f474d222f6a0a1> Tue Sep 28 11:30:37 2021 -0700 (revert of 3d6f49a56995b845 <https://reviews.llvm.org/rG3d6f49a56995b845c40be5827ded5d1e3f692cec>) - c3717b6858d32d64514a187ede1a77be8ba4e542 <https://reviews.llvm.org/rGc3717b6858d32d64514a187ede1a77be8ba4e542> Tue Sep 28 21:00:47 2021 +0200 (reland, introduced kernel breakage) - 0f0e31cf511def3e92244e615b2646c1fd0df0cd <https://reviews.llvm.org/rG0f0e31cf511def3e92244e615b2646c1fd0df0cd> Mon Oct 4 22:26:25 2021 +0200 (fix forward) ================ 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(); ---------------- aaron.ballman wrote: > 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? What do you mean? I'm sorry, I don't quite follow. ================ 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. ---------------- aaron.ballman wrote: > 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. Sorry, I don't quite follow whether your approving of the current approach or dismissive? 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
