Avoid extra variable

Thanks Peter. I'll wait for another day for further comments, and if none, will submit this tomorrow morning (pacific time).

In D65550#1610801, @efriedma wrote:

This only affects inline asm, it looks like, not general instructions? (I looked briefly, and it looks like instructions go through AArch64InstPrinter::printOperand instead.)

I don't think this would cause any issues off the top of my head, but you might want to run something like an Android build to confirm that.

Thanks for the workaround @peter.smith. I suggested it to the NDK developer as well. The immediate need for this change is gone, but I agree that this is useful for the various reasons you outlined.

Apply clang-format

Call strlen(gcov_prefix) only if its not null.

Remove stray new line.

Refactored the loop to call getenv just once.

LGTM for Android.

Skip version script for all MinGW builds, not just when using lld.

In D63743#1556912, @mstorsjo wrote:

I'd suggest skipping the lld condition, the version script adds no value here.

For ELF, version scripts do two things, select what symbols to export using glob pattetns, and set symbol versions on them.

As COFF doesn't have symbol versions, all it does on mingw with GNU ld, is filtering symbols, to export *. So it adds no value there.

In D54125#1313836, @void wrote:

Hi @piramam,

This change is breaking the Linux ThinLTO build. I'll work to get you a test case. Could you revert this patch until we can resolve the issue?

Someone just pointed out to me that even in Android P [1], libm's sincos() just called sin() followed by cos() so this optimization won't be beneficial. It was changed in March 2018 [2] but is not a part of any release yet.

In D54125#1291931, @smeenai wrote:

From the summary,

"In ELF, symbols non-weak symbols"

The first "symbols" seems extraneous?

Thanks for the review Teresa. I'm running the tests again after a rebase and will commit this after the tests pass.

Fix a few more comments.

Update based on review comments. Hopefully I did the WeakForLinker->Prevailing
name changes in all the right places.

Rename thinLTOResolveWeakForLinker* to thinLTOResolvePrevailing*

Fix commit message.

Adding reviewers suggested by 'arc cover'.

Simplify patch.

This file is not built for WIN32.

As I mentioned in the discussion, we decided to carry build rules for the proposed library in downstream. I've updated this to make it more general, and will leave it open in case there's more interest to revive it in the future.

Add empty source file to silence CMake warning.
Support more platforms, similar to libLLVM.so

Thanks for the clarification Richard and Eli. I agree that leaving the status quo will match the intent of the macro. I'll abandon this.

This implements the new library proposed in http://lists.llvm.org/pipermail/cfe-dev/2018-August/058736.html.

Sorry this fell of my radar. I've rebased the patch.

Rebase

In D50199#1186164, @rnk wrote:

Does this do anything other than -DUNICODE? Maybe just translate it at the driver level and skip the -cc1 flag?

Ping...

Ping...

In D44532#1039682, @pirama wrote:

In D44532#1039666, @phosek wrote:

Can you elaborate on the further issues with cross-compiling libcxx without a libcxxabi you mentioned in your commit message? I'd like to improve the build to handle all cases.

The built libcxx had references to some symbols from libcxxabi (__cxa_guard_acquire, I think). I'll reproduce the error tomorrow and report back.

In D48402#1140046, @rnk wrote:

In D48402#1140019, @pirama wrote:

Sections:
....

3 .text$___Z3fooi 00000013  00000000  00000000  0000012c  2**4
               CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE, LINK_ONCE_DISCARD

Could that explain the difference?

Woops, one too many calls to getNameWithPrefix.

Reid, thanks a lot for addressing this issue.

Hi Petr, thanks for the fix (I dropped the ball on this one :( ). Instead of duplicating the checks, does it make sense to fold check into addArchSpecificRPath and rename it to 'addArchSpecificRPathIfRequested' or something similar?