This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9ba8c4024b52: Fix behavior of ifuncs with used
extern C static functions (authored by erichkeane).
Herald added a project: clang.
Changed prior to
tahonermann accepted this revision.
tahonermann added a comment.
This revision is now accepted and ready to land.
Looks good to me! I left one final comment about a double period that was my
fault.
Comment at: clang/lib/CodeGen/CodeGenModule.h:1578
+ /// may not reference
erichkeane updated this revision to Diff 419840.
erichkeane marked 2 inline comments as done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122608/new/
https://reviews.llvm.org/D122608
Files:
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CodeGenModule.h
erichkeane marked 2 inline comments as done.
erichkeane added inline comments.
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6393
+// If Val is null, that implies there were multiple declarations that each
+// had a claim to the unmangled name. In this case, generation
tahonermann added inline comments.
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6393
+// If Val is null, that implies there were multiple declarations that each
+// had a claim to the unmangled name. In this case, generation of hte alias
+// is suppressed. See
erichkeane added inline comments.
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6390-6391
llvm::GlobalValue *Val = I.second;
-if (Val && !getModule().getNamedValue(Name->getName()))
+llvm::GlobalValue *ExistingElem =
+
erichkeane updated this revision to Diff 419741.
erichkeane marked 9 inline comments as done.
erichkeane added a comment.
Fix all the things @tahonermann found.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122608/new/
https://reviews.llvm.org/D122608
Files:
tahonermann added inline comments.
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6348-6349
+} else {
+ // If neither of these things, we have a user we don't know how to
handle,
+ // so default to previous behavior of emitting a terrible error message.
+
erichkeane updated this revision to Diff 419530.
erichkeane added a comment.
fix a typo in the 'triple' .
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122608/new/
https://reviews.llvm.org/D122608
Files:
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CodeGenModule.h
erichkeane updated this revision to Diff 419526.
erichkeane marked an inline comment as done.
erichkeane added a comment.
Applied clang-format, removed 'Name' as Aaron found is no longer used, fixed
the Sema test to not run on Windows triple, where ifunc is not available
CHANGES SINCE LAST
aaron.ballman added a comment.
FWIW, I'm seeing a precommit CI failure on Windows:
Failed Tests (1):
Clang :: SemaCXX/externc-ifunc-resolver.cpp
May as well also fix up the clang-format issues in the review.
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6319
+bool
erichkeane added a comment.
Ping!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122608/new/
https://reviews.llvm.org/D122608
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
erichkeane updated this revision to Diff 419143.
erichkeane added a comment.
Going through @tahonermann 's comments to attempt to allow multiple ifunc
references, I ended up down the rabbit hole.
Turns out there are ways for there to be a ConstantExpr bitcast in the way that
needs to be
erichkeane added inline comments.
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6323-6325
+ // If there is more than 1 use, I don't really know what we can do.
+ // EmitStaticExternCAliases won't be able to do anything, and the ifunc
+ // diagnostic will have to catch this.
tahonermann added inline comments.
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6319
+/// If all uses of the GV are from an IFunc resolver, which can happen when the
+/// IFunc resolver is a static-function, but the name ends up being different,
+/// return the IFunc so it
erichkeane updated this revision to Diff 418682.
erichkeane marked an inline comment as done.
erichkeane added a comment.
Do the two comment fixups aaron suggested.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122608/new/
https://reviews.llvm.org/D122608
Files:
aaron.ballman added a reviewer: rjmccall.
aaron.ballman added a subscriber: rjmccall.
aaron.ballman added a comment.
CodeGen is not my wheelhouse, so I've added @rjmccall in case he spots
something I've missed. This seems reasonable to me though.
Comment at:
erichkeane created this revision.
erichkeane added reviewers: tahonermann, aaron.ballman, cor3ntin.
Herald added a project: All.
erichkeane requested review of this revision.
We expect that `extern "C"` static functions to be usable in things like
inline assembly, as well as ifuncs:
See the bug
18 matches
Mail list logo
