LGTM

$triple-clang, also

LGTM

lgtm

LGTM. Thanks for fixing this!

LGTM

the targets where you would want to use this can't run the libcxx

testsuite anyway (because they don't have an operating system to run the
test programs under).

Is it worth adding -Werror=zero-as-null-pointer-constant to the build?

Should we go with current patch?

LGTM

You can avoid the env var by adding a substitution for opt in the lit.cfg.py with the extra argument.

LGTM

I say that because there are contexts where it is absolutely critical that https://xkcd.com/221/ not be the implementation of an RNG.

I'd much rather provide no implementation than one that lies. Broken builds are much safer than problems at runtime.

In D28820#944365, @efriedma wrote:

What is the best way to modify the code for this compiler change ?

Currently, the "interrupt" attribute only has an effect on functions, not function pointers, so your code won't work the way you want. It's a bug that we don't emit a warning for this.

I think just remove it from them.

(i.e. you should delete that, since this is the last dependency on it AFAICT)

There's a: set(TARGET_TRIPLE ...) in each of these runtimes' CMakeLists.txt...

looks fine to me, but this is the sort of thing that @EricWF usually wants the final say on.

In D40734#942352, @beanz wrote:

The docs changes look good to me. I think if we want to deprecate llvm/projects for runtime libraries, we should have a discussion on llvm-dev & cfe-dev. I think too few people are using llvm/runtimes today as part of their regular builds for me to feel comfortable deprecating llvm/projects without giving some notice first and getting more people to try the runtimes directory.

LGTM

What's the advantage to using python for this over doing it directly in CMake?

In D39534#915937, @mstorsjo wrote:

In D39534#915935, @jroelofs wrote:

In D39534#915920, @mstorsjo wrote:

In D39534#915916, @compnerd wrote:

Very well, if thats the current implementation in the AsmParser, thats reasonable. I don't think that the directive has anything to do with the file format though.

I can agree with that. In addition to making the assembler accept/reject certain instructions though, it actually does another thing which actually is file format specific - it sets the eabi attributes that indicates that the object file contains such instructions.

Are they eabi/gnueabi things?

I think so. You can read them with readelf -A foo.o, and override them with manual .eabi_attribute attributes. (That's useful e.g. for indicating that while a binary contains NEON instructions, it doesn't strict require them for running. E.g. raspbian does check such tags for checking that all binaries work on their baseline of armv6.)

In D39534#915920, @mstorsjo wrote:

In D39534#915916, @compnerd wrote:

Very well, if thats the current implementation in the AsmParser, thats reasonable. I don't think that the directive has anything to do with the file format though.

I can agree with that. In addition to making the assembler accept/reject certain instructions though, it actually does another thing which actually is file format specific - it sets the eabi attributes that indicates that the object file contains such instructions.

In D39146#902647, @compnerd wrote:

The name is a bad idea, because you could get an expression or a value which is not a region name. I think that you should test that as well.

Needs a docs entry for the new flag (in libcxx's BuildingLibcxx.rst). Other than that, all the stuff I've asked you to add LGTM. I'd still appreciate @EricWF / @mclow 's opinion on the meat of the functional change part of this though... I don't know all the implications relaxing the search here.

LGTM

In D38599#893990, @jroelofs wrote:

In D38599#893985, @danalbert wrote:

In D38599#893903, @jroelofs wrote:

That reminds me... this does need a testcase or two.

Oh, also, any test I add is going to fail, since the case I'm trying to account for here is not the default behavior.

That's what an available_feature + // REQUIRES: is for.

In D38599#893985, @danalbert wrote:

In D38599#893903, @jroelofs wrote:

That reminds me... this does need a testcase or two.

Oh, also, any test I add is going to fail, since the case I'm trying to account for here is not the default behavior.

That reminds me... this does need a testcase or two.

Where is it used?

LGTM

I'm not sure I'm the right person to review this.

Add a testcase per @bogner's advice (sort of). I tried doing the setenv trick, but that only worked when the testcase is ran not under LIT. Since I have to inject other state in anyway for testing, may as well just use that flag to force 60 cols too.

Rate-limit it, since it'll look awful if you call update() really often.

It occurs to me that Chandler's also the right person to ask about ideas on how to estimate the progress of a PassManager.

@aaron.ballman suggested adding @chandlerc as a reviewer

r313680

I'm not sure it's better than writing the if/elseif/elseif/elseif out explicitly :/

Unless you're going for "make this lit.cfg as close as possible to clang's", then...

r312748

r312651

Sure. I'll commit it for you once this build/test cycle is finished.

FWIW, I defaulted this to -mthread-model=single to match existing baremetal gcc toolchains, and support armv4t & others that don't have the requisite atomic ops.

LGTM

Does the unwinder need this too?

r309147

In D35880#821352, @mgorny wrote:

The code looks good, and seems to work. I'm sometimes getting a failure from shtest-timeout.py but I suspect that's an irrelevant race condition.

LGTM

In D34918#801611, @weimingz wrote:

@EricWF , do you mean factoring out the __sync_* functions like this?

r307266

@EricWF we don't do any installed testing, only testing from the build dir, right?