maybe we can apply the -Wweak-vtable flag lib by lib. does the CMakeList support it?

In D45244#1064905, @dblaikie wrote:

Great - thanks for sticking with it, sorry for my dodgy explanations!

If you're adding in anchors, are you using -Wweak-vtables to find them? If you're pushing through to make LLVM (& hopefully Clang/other LLVM subprojects) -Wweak-vtables clean, perhaps you can turn on the warning in the CMake config once it's clean so we don't regress this again? (it'd also be interesting to do some kind of analysis to see whether avoiding weak vtables is /actually/ worthwhile - like maybe making a temporary/local/non-committed change to change all the explicit anchors into inline functions (unanchoring them) & see if builds are much larger or longer, etc)

split the original into two parts. This one supports -ffile-macro-prefix-to-remove function.

Sure. How about do it in a separate patch?

@dblaikie you're right. I modified CMakeLists.txt rather than LLVMBuild.txt

In D45244#1064649, @dblaikie wrote:

That sort of surprises me - if that were the case, there would be no point
having LLVMBuild.txt files in any libraries, I would think?

In D45244#1064527, @dblaikie wrote:

I meant what happens if you remove the changes to tools/lto/LLVMBuild.txt
and tools/llvm-lto/LLVMBuild.txt and /only/ have a change to
lib/LTO/LLVMBuild.txt - is that sufficient to make the build work?

I put a dependency on tools/llvm-lto as well. From my build (default CMake config), it works.

add dependency upon MCJIT for LTO due to the use of ObjectMemoryBuffer

If I choose to define ObjectMemoryBuffer::anchor() in MCJIT.cpp (the module that uses ObjectMemoryBuffer), then I got error like:
../../lib/libLLVMLTO.a(ThinLTOCodeGenerator.cpp.o): In function `ObjectMemoryBuffer':
llvm/ExecutionEngine/ObjectMemoryBuffer.h:41: undefined reference to `vtable for llvm::ObjectMemoryBuffer'

Thanks Eli!

When I statically linking a project (built with RTTI) against LLVM OrcJIT (default built with no-RTTI), I got a couple of "undefined reference to type info" error.

gentle ping?

ping?

Modified the random generator in filesystem_test_helper to use high_resolution_clock as seed.

ping?

Disable tests that depend on random_device.
filesystem tests rely on random_device as seed to create random path. Although it's possible to avoid the random_device but if the build target has no random_device, it's very possible that neither filesystem nor other seeding device like clock is available.

As Sam suggests, add more checks in lit tests to make sure only "bl" is used.

On Thumb1, i64 shift is lowered to 20 instructions. For example:
For code like
unsigned long long foo(unsigned long long x, unsigned y) { return x << y;}

any suggestions?

We can wrap the random_device as a minstd_rand, a linear congruential enginer that a lot of C lib uses for rand().
However based on documentation, we should just provides dummy implementation which throws an exception in the constructor of random_device [1,2]
But compared with run-time exception, a link time error is better if we simply skip the implementation. Any thoughts?

Should we go with current patch? or provide a srand/rand based implementation as an option?

Does it make sense to provide an implementation based on C99 rand?
like
#ifdef _LIBCPP_USING_C99_RANDOM

srand(0), rand()

In D40245#940424, @fedor.sergeev wrote:

ping?

In D40269#930857, @weimingz wrote:

Hold on. Why do we need this flag? There is no c++ code in builtins.

Hold on. Why do we need this flag? There is no c++ code in builtins.

LGTM. Adding Saleem and Peter for confirming.

If compiler-rt is to provide optimized version, then, should MUSL just forwards the calls like memcpy ->__aeabi_memcpy ? Because libc is usually appear first in linking order, a standard implementation in libc will hide the optimized version in compiler-rt.

In D39700#917653, @peter.smith wrote:

I agree that we must avoid the 16-bit branch on Thumb1 and that the code sequence is fine for that purpose.

I do wonder if compiler-rt should define the aeabi_... forwarding functions in it? I would have expected the C-library to define them. For example the v6-m libgcc.a does not define the aeabi memcpy family, the forwarding is done in newlib. Similarly Arm's proprietary compiler's C library defines aeabi_memcpy that just falls through directly into memcpy. I'm guessing that there is some C-library that we support that does not define the aeabi_ function?

add memcpy

Should we use "ARM" instead of "Arm" ?

LGTM. (Let Saleem to sign off)

LGTM.

minor change

Moved the inclusion from stdexcept.cpp into refstring.h

LGTM. Adding Renato and Saleem to approve.

guard the "DEFINE_CODE_STATE" with "__ARM_ARCH < 6" check.

ping @EricWF

remove unreleated change (CMakefile) and address comments from Saleem

In D35970#840370, @olista01 wrote:

I think it would be better to reject these options than to silently accept them, unless you plan on using them to emit build attributes.

ping?

And since it's an assertion, in Release build, the flag will still be accepted silently.

rebased. Please review. Thanks

In D36249#830645, @saugustine wrote:

In D36249#830121, @weimingz wrote:

I tried to address it via checking pre-defined macros:
https://reviews.llvm.org/D31573

As long as the macros are defined correctly by clang, we don't need to worry about the specific target machine. How do you think about it?

I like the idea of a feature check, rather than a specific architecture check--that is clearly the right thing to do.

On the other hand, I would like to mark the test as unsupported and not run in that case, rather than running it, saying it passed, but not actually testing anything. That better reflects the state of the implementation. Unfortunately, I don't think that can be done with macro checks. So my preference would be this patch over D31573, but I would also find D31573 acceptable if it came to that.

Finally, 80-bit doubles are a bit of a historical artifact these days. Only x86 and m68k have them (and not even all m68Ks either). So I don't think it matters that much.

I tried to address it via checking pre-defined macros:
https://reviews.llvm.org/D31573

In D35970#826789, @t.p.northover wrote:

This looks like it'll get the build attributes wrong (since only PIC gets plumbed through to the backend). That's probably worse than simply rejecting the arguments.

add test case

ping?

rebased after reverting r307595 and moving __refstring header

address the review comments by @peter.smith and @jamesduley

rebased

I should use existing include/atomic_support.h. Will upload the patch shortly.

simplify test case

missed to upload the change to include __atomic_support header file