I'm generally okay with the approach of this patch. I'm not sufficiently well-versed in the clang codegen side of things to know if this covers all of the bases, and I'd appreciate someone who is familiar with that side of things to approve this patch.

I'm studiously ignoring the Clang and LLVM codegen changes here, but otherwise, I think the direction of this change is generally good.

Friendly review ping?

Documentation tweaks.

Friendly review ping?

Thank you for adding documentation for target("aarch64.svcount"). Otherwise, the only other thing I would comment is the same thing that Nikita said with regards to the unused new methods.

I'd ideally like to see discussion of this target extension type somewhere in documentation, but alas, AArch64 is one of the backends that lacks a dedicated target documentation page.

Mostly test fixes.

In D142907#4132430, @arsenm wrote:

I was thinking of changing the default in general to dynamic. I was going to at least change the strictfp default in a follow up

Not entirely sure where the best place to effect this (I think somewhere in the clang driver code?), but on further reflection, it feels like strict fp-model in clang should set the denormal mode to dynamic.

This updates code, and rebases tests on top of trunk.

Looking at the attribute logic here, there is conceptual room for both a dynamic and an unknown mode (i.e., you get a top and a bottom value), but I don't think there is value in distinguishing between them, so I'm fine with keeping just a dynamic.

In D137811#4095594, @sepavloff wrote:

What about the original case, when isnan is used in inline function?

inline int get_code(float x) {
  return isnan(x) ? 1 : 2;
}
#pragma STDC FENV_ACCESS ON
void func2(float x) {
  int code1 = get_code(x);
  ...
}

Can this function, defined as non-strictfp, raise Invalid exception when inlined into strictfp function?

In D137811#4085810, @arsenm wrote:

For the purpose of this change, the exact definition of FENV_ACCESS doesn't matter. This is not something that the context of a single instruction can or should consider. This would have to be managed by implicit mode switches inserted by the frontend to maintain the invariant that non-strict functions don't have to be concerned about it. Our strictfp representation would be totally broken if we had to think about this in a non-strict function

In D137811#4084663, @sepavloff wrote:

What about such code?

int get_code(float x) {
  return isnan(x) ? 1 : 2;
}
void func1(float x) {
  int code1 = get_code(x);
  ...
}
#pragma STDC FENV_ACCESS ON
void func2(float x) {
  int code1 = get_code(x);
  ...
}

Acoording to C standard it must work as intended. If compiler replaces isnan with comparison, the code would be broken.

In D137811#4081771, @sepavloff wrote:

In D137811#4068836, @kpn wrote:

If exceptions were turned off, the call was made, and exceptions were turned back on then there would be no correctness issue. Inlining the called function wouldn't change that. The toggling of the exception enablement would be invisible to LLVM. Thus we can't categorically say that a strictfp function calling a !strictfp function is malformed.

On most targets turning off FP exceptions means reading content of FP control register, changing value of mask bits and putting the modified register value back. It is expensive operation and cannot be made invisible to LLVM. Some targets (like RISCV) do not have possibility to mask FP exceptions at all, so at IR level there is no way to turn exception off. Default FP environment supposes that the exceptions are ignored, not disabled. In general case they are raised always.

From some of the verifier checks and tests, it looks like target("x86.amx") would also require some new type properties, to express its unsuitability for alloca-and-friends, as well as non-intrinsic arguments.

Fix some of the code review comments.

In D135202#4038774, @zixuan-wu wrote:

I'd like to enable the backend infra and propose draft later. To better demonstrate and enable backend target-independent code completely, I'd like to demonstrate with x86_amx that changing x86_amx type to target extension type and make all test passed, if nobody objects. @jcranmer-intel @LuoYuanke @craig.topper @pengfei

In D135202#4022488, @zixuan-wu wrote:

hi, @jcranmer-intel , could you please propose a backend demo revision to demonstrate how to use targetext type of backend?

Most of the testing for this change has been in conjunction with the changes in the SPIRV-LLVM-Translator repository here: https://github.com/KhronosGroup/SPIRV-LLVM-Translator/pull/1799. I haven't updated the in-tree experimental target to support the target-extension types, but I did want to provide more documentation as to how these types work in the existing LLVM documentation for the SPIR-V backend.

There might be some cases where we can maintain some fast-math flags that would otherwise be dropped... but it's almost certainly not worth the bother here, since I expect the vast majority of cases will have all the relevant operations have the same fast-math flags.

In D135202#4008212, @barannikov88 wrote:

@jcranmer-intel
A new build warning:
llvm/lib/Target/Hexagon/HexagonTargetObjectFile.cpp:296:11: warning: enumeration value 'TargetExtTyID' not handled in switch [-Wswitch]

Fix typo in release notes.

Address review comments (and clang-format issues).

Rebase to trunk, and add release notes.

In D135202#3996159, @aeubanks wrote:

the comments and patch description are inconsistent about "opaque" vs "target extension", cleaning this up would be good for avoiding confusion in the future

Documentation and test tweaks.

Address review comments.

In D139316#3984954, @sepavloff wrote:

PPC uses double double encoding (https://en.wikipedia.org/wiki/Quadruple-precision_floating-point_format), in which a number is represented by a sum of two double precision numbers, where the second just keeps additional mantissa bits. Total number of mantissa bits in ppc_fp128 is about 107 bits. Exponent range is the same as for double numbers: 2^-1022 to 2^1023. So large numbers (above 2^108) are always integers, they do not need rounding and the result of ceil, floor etc is the argument.

In D139316#3981280, @kpn wrote:

Are we certain that a value that is very close to infinity won't trip over into infinity after rounding?

I don't normally handle name mangling, so I can't comment too much here, but I will note that Itanium ABI is planning on using DF16b for std::bfloat16_t: https://github.com/itanium-cxx-abi/cxx-abi/pull/147

Friendly review ping?

If DAZ mode is semantically "denormals are non-canonical zero", then isfpclass(denormal, Zero) arguably should be true in DAZ mode, so lowering that to fcmp 0.0 would be legal. On the other hand, the actual lowering of an unoptimized isfpclass is likely to rely on bit patterns which would ignore the current dynamic value of DAZ. On the other other hand, there's no clear indication of what isfpclass returns for noncanonical values... and on the other other other hand, leaving the result of isfpclass as unspecified behavior for noncanonical inputs is incredibly unfriendly to users.

Rebase on trunk.

This strikes me as correct, but I would prefer someone more expert in PowerPC specifically to comment on this.

Doc tweaks, name tweaks, add some verification code.

Major update.

In D136861#3890025, @aemerson wrote:

Ah ok, that's fine then. Although from looking at D135202 it doesn't look like the design has been settled yet? I don't think we should be writing patches upon speculative changes, so unless D135202 is going to land very soon I'd prefer we continued with the traditional approach.

I'm similarly skeptical that this is the best way to go about the problem. Making typed pointers qualify under isPointerTy() potentially breaks a lot of code that assumes isPointerTy() means that it is indeed a PointerType, although this is ameliorated to some degree by the hard prohibition llvm::Value has on construction types with TypedPointerType.

In D136568#3878104, @Izaron wrote:

The online documentation (https://en.cppreference.com/w/cpp/numeric/math/ilogb) says:

1. If the correct result is greater than INT_MAX or smaller than INT_MIN, FE_INVALID is raised.
2. If arg is ±0, ±∞, or NaN, FE_INVALID is raised.
3. In all other cases, the result is exact (FE_INEXACT is never raised) and the current rounding mode is ignored

The first point seemingly never occur, because llvm's ilogb return type is int.
The second point is handled as expected (APFloatTest.cpp checks it)

In D135202#3860361, @nikic wrote:

I couldn't find where this has been pointed out, could you please link me to it? I don't really get why it wouldn't be feasible, given that your current proposal effectively already does it, just with a hardcoded ptr type.

Fix some of the nits.

In D135202#3855371, @nhaehnle wrote:

Here's a random thought: what if the types are parameterized using metadata instead of ints?

In D135446#3852407, @ldionne wrote:

The libcxxabi copy is the canonical version, right? If so, we could add a test inside LLVM that ensures that re-syncing the files does not produce any difference in output.