In D146148#4220495, @aaron.ballman wrote:

In D146148#4220433, @zahiraam wrote:

In D146148#4220268, @rjmccall wrote:

Okay, so modifying math.h to use this attribute is acceptable? That's great, that's definitely the best outcome for the compiler.

Just a minor request about the diagnostic name, but otherwise LGTM.

Yes. Added the attribute inside the included math.h header file.

How does this work for, say, glibc, musl, or MSVC CRT? Won't those math.h headers lack the attribute and thus run into problems when used with Clang?

In D146148#4220268, @rjmccall wrote:

Okay, so modifying math.h to use this attribute is acceptable? That's great, that's definitely the best outcome for the compiler.

Just a minor request about the diagnostic name, but otherwise LGTM.

In D146148#4213475, @rjmccall wrote:

The user isn't modifying the float_t type definition, they're using it. I think the diagnostic should say something like cannot use type 'float_t' within '#pragma clang fp eval_method'; type is set according to the default eval method for the translation unit.

Is there any way we can infer an attribute for these typedefs when they're declared, then diagnose it in DiagnoseUseOfDecl? Some sort of "available_only_in_default_eval_method" attribute?

In D145704#4184584, @aaron.ballman wrote:

LGTM! If you don't hear otherwise from @kito-cheng, @bjope, @Wilco1 in the next ~4 hours, feel free to land.

In D145704#4182268, @aaron.ballman wrote:

Thank you for working on this! Because this is fixing an ABI break, if we can land this in time for the release candidate I think we should try to add this to 16.0 to limit damage. CC @tstellar for his opinion.

This doesn't seem to fully revert bbf0d1932a3c1be970ed8a580e51edf571b80fd5; that commit changed clang/lib/Sema/Sema.cpp but this commit does not back those changes out. Is that intentional?

Tagging @davemgreen, @kito-cheng, @bjope and @Wilco1 for awareness.

Would be nice to have feedback on this! So much work was put into it! @rnk, @efriedma

HasFloat16 is unconditionally true for Arm and only gated on SSE2 support for X86

My understanding of that is because all aarch64 implementations support ARMv8 FP which makes half type for this target legal. Setting both HasLegalHalfType and HasFloat16 makes sense.
That's not the case for x86. HasLegalHalfType needs the avx512fp16 feature to be set.

ping?

In D142583#4083251, @bader wrote:

LGTM. Thanks!

Thanks. Please see my comments/questions.

@thakis I think this should fix the clang-cl use with the Mac OS compiler.

In D141929#4069763, @thakis wrote:

This breaks tests on macOS: http://45.33.8.238/macm1/53215/step_7.txt

Please take a look and revert for now if it takes a while to fix.

ping.

In D137107#3999681, @efriedma wrote:

Not sure exactly what code that generates in its current form, but that's roughly the right idea, yes.

This uploaded patch is a draft (LIT tests fails but are not updated since I am not sure about these changes) that I am part way confident about. There are still a lot of things that I need to look at, but would you mind looking at it to see if this is moving in the right direction?

Can you please be more verbose in your summary? This change is related to the use of -ffp-exception-behavior=strict right?
This attribute is set here https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CodeGenFunction.cpp#L987 but under a condition. Is that condition always satisfied when the attribute needs to be set?

@eli.friedman Thanks.

In D138117#3963560, @kawashima-fj wrote:

Addressed to @zahiraam's comment

In D137107#3931259, @efriedma wrote:

There are multiple notions of "constant" here:

• Whether a value is invariant at runtime.
• Whether a value is legal in a "constexpr" expression.
• Whether we can emit a constant without emitting code that runs at startup to initialize it.

The third is generally a superset of the second; most targets have relocations that allow us emit the address of a global variable into another global variable. dllimport variables are an exception: even though the address is constant, we can't emit a relocation.

To make this work, what we need to do is:

• Extend the notion of a "constant" in the AST/Sema to allow dllimport variables. (There should be code specifically checking for dllimport variables; we just need to drop that check.)
• Teach IR generation (clang/lib/CodeGen) how to emit constants that don't have a relocation. This probably means a check that if we're generating the initializer for a global variable, and the initializer refers to the address of a dllimport variable, we instead emit a global constructor. (Maybe with a higher priority that normal global constructor, so the user can't accidentally observe an uninitialized variable? I forget if MSVC actually has constructor priorities...). Or I guess as an initial patch, we could just emit an error if we detect this case.

Looks good to me after the additional RUN line mentioned in my comment for Driver/fp-model.c is added. Thanks.