[PATCH] D46847: [WebAssembly] Move toString helpers to BinaryFormat. NFC.

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added a comment.

In https://reviews.llvm.org/D46847#1101983, @vsk wrote:

> In https://reviews.llvm.org/D46847#1101801, @sbc100 wrote:
>
> > I can't reproduce those failures locally.   I also build with BUILD_SHARED_LIBS=ON so I would expect to see the same errors as you.
>
>
> I don't use that option, but I do use LLVM_ENABLE_MODULES=On.
>
> > Can you see how RandomAccessVisitorTest.cpp makes use of BinaryFormat/Wasm.h?
>
> In the .o, I see:
>
>   __ZNSt3__110unique_ptrIN12_GLOBAL__N_123RandomAccessVisitorTest15GlobalTestStateENS_14default_deleteIS3_EEED1Ev:
>   ...
>   000000000000032a        movq    __ZNK4llvm6object10WasmSymbol4dumpEv(%rdi), %r14 ## llvm::object::WasmSymbol::dump() const
>
>
> For some reason, the unit test's GlobalTestState deleter needs to store a function pointer to WasmSymbol::dump? If I remove "LLVM_DUMP_METHOD" from the definition, it no longer appears in the .o. I think something strange with  __attribute__((__used__)) is happening, and I don't understand why. The only use of this symbol in the IR is in the magic @llvm.used global.
>
> >   Your fix looks find to me though.  Is it just those two targets that fail?
>
> These are the only two targets which fail to build as a part of 'check-llvm'. Do you want to dig any deeper into this, or should I just land the build fix?


Yes, please land the fix.  Its a logical extension to the fixes I already made.  I'd try with LLVM_ENABLE_MODULES out of curiosity too.


Repository:
  rL LLVM

https://reviews.llvm.org/D46847