[PATCH] D118575: [llvm-libtool-darwin] Fix crash with bitcode asm module

Thu Feb 3 13:17:57 PST 2022

smeenai accepted this revision.
smeenai added a comment.
This revision is now accepted and ready to land.

LGTM since we're just matching llvm-ar's logic.

================
Comment at: llvm/test/tools/llvm-libtool-darwin/asm.test:1
+## This tests that archives are correctly created when the llvm
+## has native assembly info
----------------
keith wrote:
> smeenai wrote:
> > I think you'll want `REQUIRES` lines for the targets you're using here (`x86` and `aarch64`), to make the test not fail when LLVM is built without those targets. (I think just testing one architecture would be fine as well, to not require multiple targets to be enabled for running the test).
> Ah yes thanks, I added both here. The reason I want both is because theoretically depending on how you register the targets, it may only work for your native target, and not any other targets for cross compilation, so having both here verifies that. If you think having both requires is too strict, I can split these into 2 test files?
> 
> Also note apparently in llvm/test the lit config is slightly different so the requires definitions are different here.
Yeah, that logic for testing both makes sense. I do think it'd be nice to split out into two files though (just to maximize testing coverage).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118575