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?

Done (328644). Cited in the commit message that you are the author since that gets overwritten when someone besides the author commits a change.

Thanks for the fix Yan.

Thanks for the reviews!

The changes look good to me. But something is funny in Phabricator - I am unable to see the context in this file. How did you upload this CL?

Fix comment.

Remove unexpected change from another patch.

• [CodeGen] Mark fma as const for Android

• [CodeGen] Mark fma as const for Android

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.

Change name of option to COMPILER_RT_FUZZER_LINK_LIBCXX.

In D40590#938576, @eugenis wrote:

For sanitizers, we build only for api 14 and use that binary everywhere. Why is libfuzzer special?

Thanks for the review. Now let's just hope the windows bots stay happy :)

Normalize falkor and saphira as well.

Sink 'Arg *' declaration.

Add missing space.

Address review comments.