Looks reasonable to me -- but I'd like to see the LLD change implemented and both pieces tested together end-to-end before this is submitted.

+1 Agreed that impl simplicity should be prioritized.

Also, I note the doc says it's useful for for “load address” and “push address” instructions (note, "load address" means e.g. x86 "lea" instruction) -- which should NOT be dependent upon the value stored in the memory. The x86 backend actually uses a "Ts" constraint for *lea<mode> which is defined by define_address_constraint, documented as making a constraint that is basically equivalent to p (just with different limits as to what kinds of addresses are accepted.)

‘p’ in the constraint must be accompanied by address_operand as the predicate in the match_operand. This predicate interprets the mode specified in the match_operand as the mode of the memory reference for which the address would be valid.

It looks to me from GCC that constraint 'p' is intended to be used internally, not for inline-asm. I question whether the kernel should be using it, and whether we should even implement it at all.

In D144999#4165106, @oontvoo wrote:

In D144999#4163617, @int3 wrote:

One issue is that we need to emit ONLY dwarf-unwind for those functions which use nonstandard personalities, while femit-dwarf-unwind enables emitting both compact-unwind and dwarf unwind.

So IMO the ideal design is that we emit both and have the linker fall back to DWARF unwind info if the number of personalities is too large.

OK - this sounds like a reasonable behaviour. 👍

no-compact-unwind is particularly useful for newer x86_64 platforms: we don't want to omit DWARF unwind for x86_64 in general due to possible backwards compat issues, but we should make it possible for people to opt into this behavior if they are only targeting newer platforms.

So, sigh -- I've just realized this will need a (small) linker change, as well. Currently, we (both LLD and LD64) encode the personality function in the compact-unwind even for unwind infos marked MODE_DWARF.

On the other hand, I think a not-insignificant number of users are interested in freestanding environments for one-off/fun/experimental projects where ease of access is more important than performance characteristics -- think: users who are playing around with an arduino, etc.

In D144889#4156974, @rsmith wrote:

Likely because of GCC's perspective on this, the set of C headers provided by GCC, Clang, ICC, etc. has included the complete list of freestanding headers and more or less no others, with some libc implementations providing the full set and some providing just the non-freestanding ones. If we're moving away from that, we'll presumably need a new agreement between compiler folks and libc folks about which headers are provided by whom (and I expect it'll be "compilers provide what the C standard said before this change to freestanding and we pretend the change never happened").

however the fix in most cases should be to use -std=c++2b and the code will work out of the box.

(Firstly, really nobody should be using the __sync_* builtins...)

Given the reassurance regarding optimizations of non-deterministic results, I'm satisfied.

In D143074#4115297, @kpn wrote:

How many of these sNaN vs qNaN cases that matter are there? I've seen pow() mentioned in this ticket. What are the other cases?

In D143074#4108919, @sepavloff wrote:

GCC has option -fsignaling-nans, which may be used to turn on standard treatment of SNaNs. Clang does not have similar option, now the only way to handle SNaNs in the standard way is to turn on exception handling, which is not suitable in some cases. LLVM as a low-level component must support SNaN on an opt-in basis, but request for such support must be more selective. Anyway SNaN treatment is nor a part of strict exception handling semantics.

The property of SNaN like SNan + 0.0 -> QNaN is not related to exceptions and must be preserved in default environment also.

In D143074#4105090, @RalfJung wrote:

we cannot allow LLVM to spuriously introduce sNaNs when the original code did not use any.

That is one of the things being discussed here, right? Right now at least on old MIPS LLVM does *not* satisfy this requirement (apfloat will produce sNaN for that platform). The LangRef says "No floating-point exception state is maintained in this environment" which I read as "we don't care at all about the sNaN vs qNaN distinction and will just do whatever".

As far as I can tell, this conversation has run its course, and I don't think there were any more comments on the actual code, so I'm going to accept this revision.

C standard library's semantics, which state that pow(1.0, NaN) = 1.0, for any NaN

In D143074#4104239, @RalfJung wrote:

Floating-point math operations assume that all NaNs are quiet.

That could be read as "if you produce a signaling NaN (e.g. via a bitcast) and feed that to a math operation, that violates the assumption, and hence is UB".

Overall, I think the points we need to convey are here:

1. It's assumed that the rounding mode is round-to-nearest, and all floating-point traps are disabled.
2. The status flags may be set to arbitrary values.
3. Floating-point math operations assume that all NaNs are quiet.

Thanks for the reviews!

For tiny and small code models, we use DW_EH_PE_sdata8 instead of GCC's
DW_EH_PE_sdata4. This is a deliberate choice to support personality-.eh_frame
offset > 2GiB.

Rebase.

Rebase.

Rebase.

I do not believe there's any problem with an isnan to fcmp transform -- as long as we restrict it to non-strictfp functions. And I'm not sure why there's so much debate here. I thought all those sorts of semantic questions were already pretty well settled. Does the documentation about strictfp vs not-strictfp semantics need to be made clearer?

Thanks, looks good now!

This does not matter in practice because these call site tables are all comprehensive

Rename 'cf' field to 'cond' per review request.

In D120634#4046133, @vitalybuka wrote:

This bot is broken https://lab.llvm.org/buildbot/#/builders/37/builds/19454

Fix transitive_includes test errors.

Add missing auto-generated file, fix review nits.

Now that D136554 has re-landed, it seems that we're now actually good to go here (yay!)

Rebase, rerun ninja libcxx-generate-files, address last review nits.

Hearning no objections, I'll approve this.

Leaving an xref trail here, I filed issues against the two projects I reported above:

• Apache Arrow: https://github.com/apache/arrow/issues/15281
• v8: https://bugs.chromium.org/p/v8/issues/detail?id=13648

I wonder if, at this point, we could just default relax-elf-relocations to true in LLVM -- and potentially even remove the cmake ENABLE_X86_RELAX_RELOCATIONS knob, as well.

AFAIU, the upshot of https://sourceware.org/bugzilla/show_bug.cgi?id=24784 is that -fno-plt is broken after this change, unless -mrelax-relocations is enabled. Clang enables it by default subject to the CMake option ENABLE_X86_RELAX_RELOCATIONS (which defaults to on), but it's disabled by default by LLVM itself.

Now PPC and VE are the last two targets still using useDeprecatedPositionallyEncodedOperands. This change is repetitive and large -- but necessary.

Rebase

Rebase.

So, I guess these have all been getting actually miscompiled on riscv64 up till now? Whoops!

Thanks for the change.