[PATCH] D145208: [COFF] Add MC support for emitting IMAGE_WEAK_EXTERN_ANTI_DEPENDENCY symbols

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added inline comments.


================
Comment at: llvm/lib/MC/MCExpr.cpp:764
 static bool canExpand(const MCSymbol &Sym, bool InSet) {
+  if (Sym.isWeakExternal())
+    return false;
----------------
efriedma wrote:
> bcl5980 wrote:
> > Do we need to limit it to AntiDep only? This code looks means any weak symbol can form a cycle.
> isWeakExternal should be true for any symbol where we call setIsWeakExternal, which should be all weak symbols on Windows.
> 
> The whole recursion this is just much less likely to be relevant for non-AntiDep weak symbols; I've never seen this sort of usage of aliases elsewhere.
in https://crbug.com/1435480 we're getting undefined symbol errors with symbols introduced via whole program devirtualization. removing this change resolves the issue. any thoughts?

the undefined symbol is a hidden alias to a weak_odr alias to a private ptr array constant


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145208/new/

https://reviews.llvm.org/D145208