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

Tue Mar 14 13:49:36 PDT 2023

mstorsjo added inline comments.

================
Comment at: llvm/test/MC/COFF/alias.s:27
 // CHECK:        0x8 IMAGE_REL_I386_DIR32 global_aliased_to_local
-// CHECK:        0xC IMAGE_REL_I386_DIR32 external2
+// CHECK:        0xC IMAGE_REL_I386_DIR32 weak_aliased_to_external
 // CHECK:      ]
----------------
efriedma wrote:
> mstorsjo wrote:
> > This looks curious - why does this change here?
> We were resolving ".long weak_aliased_to_external" to external2; the new isWeakExternal() checks prevent us from resolving that, I think.  I wasn't really trying for that, specifically... but I think it's the behavior we want?  (If a function refers to weak_aliased_to_external, we should allow it to be overridden whether or not the reference is in the same object file.)
Ah, I see - yeah, that makes sense and sounds like a desireable result indeed.

================
Comment at: llvm/test/MC/COFF/weak-anti-dep.s:23
+// CHECK-NEXT:      Linked: b (8)
+// CHECK-NEXT:      Search: 0x4
+// CHECK-NEXT:    }
----------------
efriedma wrote:
> mstorsjo wrote:
> > I guess it'd be good to teach `llvm-readobj` about this new mode too.
> You mean you want a readable name for the "0x4"?  Not sure where that comes from, but I can look.
Yeah, llvm-readobj has got a list of its own.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145208