[PATCH] D69064: [ARM] Add dependency on GlobalISel for unit tests to fix shared libs build

Fri Oct 18 06:35:37 PDT 2019

nemanjai marked an inline comment as done.
nemanjai added a comment.

In D69064#1714348 <https://reviews.llvm.org/D69064#1714348>, @peter.smith wrote:

> The references are coming from ARMSubtarget which is instantiated in MachineInstrTest.cpp. These have in the ARMSubtarget.h header file:
>
>   /// GlobalISel related APIs.
>   std::unique_ptr<CallLowering> CallLoweringInfo;
>   std::unique_ptr<InstructionSelector> InstSelector;
>   std::unique_ptr<LegalizerInfo> Legalizer;
>   std::unique_ptr<RegisterBankInfo> RegBankInfo;
>   
>
> There are code-generation differences in GCC and Clang. GCC generates the destructors for RegBankInfo and Legalizer in Comdat groups and calls them from the destructor of ArmSubTargetInfo, whereas LLVM does not. I don't intuitively know why there is this difference in code-generation.
>
> In the static build libLLVMGlobalISel.a is included in the link line whereas it isn't for the shared build. I think that unless there is a good reason for GCC not to output the destructors then it is worth adding GlobalIsel.

We don't really have control over how the GCC build compiler works, so I really think we should go ahead with this patch. This is particularly important since the PPC bot has also been failing for 3 days or so with the same error (caused by r374887):
http://lab.llvm.org:8011/builders/clang-ppc64le-linux-multistage/builds/10972

I initially didn't realize that this is causing build bot breaks, so I plan to commit this patch today to get the bot back to green.

================
Comment at: unittests/CodeGen/CMakeLists.txt:8
   Core
+  GlobalISel
   MC
----------------
This one is actually not needed. Just the other one. So on the commit, I'll commit just the change in `unittests/Target/ARM`.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D69064