Fix few format problems.

Thanks for the review!

Address review comments.

Add signaling intrinsic handling.

In D69281#1771143, @uweigand wrote:

In D69281#1770668, @pengfei wrote:

Adding an extra argument to the DAGnode (sort of like FP_ROUND) also need special code all over the place to handle ...

Hi @uweigand, I don't quite understand why we need special code all over the place to handle. From what I can see, STRICT_FSETCCS always have the same action with STRICT_FSETCC in common code.

I had the variant with the extra operand implemented initially, and what I was seeing was that made the node "special" because now we don't have the same set of operands between SETCC and STRICT_FSETCC any more. Usually, the STRICT_ opcodes have the same set of operands as the regular operands, with the exception of the chain. Code tends to assume that this is the case (e.g. when morphing a strict node into a regular node, or when writing a single legalization or other transformation that applies to both the strict and the regular node). Having not just the chain, but one additional extra operand required code changes to handle that operand in various places. Instead, with two DAG opcodes, you usually just have to add two more cases to a switch and that's it.

But those were really minor issues; the primary reason for me to switch was that I needed a way to tell common code which operations are legal, where the ISA has differences between signaling and quiet operations. (On Z we always have both for scalar types, but for vector types we got the signaling operations at a later ISA level than the quiet ones). This is trivial with two opcodes, but would require quite a bit of custom code if when using just a single one.

Adding an extra argument to the DAGnode (sort of like FP_ROUND) also need special code all over the place to handle ...

Hi @uweigand, I don't quite understand why we need special code all over the place to handle. From what I can see, STRICT_FSETCCS always have the same action with STRICT_FSETCC in common code.

Please note that currently we only state the rounding modes and the IEEE masks bits of MXCSR. This means instructions like VRSQRT14PS, which only have dependence on FTZ and DAZ, wouldn't be modeled.

FTZ and DAZ are modeled now by commit rGc8995de06994. Thanks.

Address review comments.

Address review comments.

Add test case.

In D70881#1766211, @craig.topper wrote:

In D70881#1766203, @pengfei wrote:

In D70881#1765349, @craig.topper wrote:

I think SAE instructions and embedded rounding instructions still read the DAZ and FTZ bits from MXCSR

I think so, but currently we don't model DAZ and FTZ, right?

The comment in X86RegisterInfo.td says that, but where was that discussed?

Thanks and sorry for the mistake.

Thanks for the review!

We make sure that every register that is used by an instruction in the snippet is initialized. This is to avoid having fluctuations in measurements due to performance depending on values in registers. I think it's great if SSE/AVX instructions start explicitly state their deps on MXCSR, because the behaviour does indeed depend on the value of these flags.

Thanks for the explanation. Please note that currently we only state the rounding modes and the IEEE masks bits of MXCSR. This means instructions like VRSQRT14PS, which only have dependence on FTZ and DAZ, wouldn't be modeled.

Address review comments.

Removed MXCSR and mayRaiseExceptions from SAE opcodes.

I'm not sure I understand why this needs to be initialized. Why don't we need to do it for FPCW?

Fix typo.

LGTM.

In D70582#1759252, @uweigand wrote:

@pengfei : I've updated the D69281 patch, which will require follow-on changes here. In particular, signaling comparisons (STRICT_FSETCCS) need to be handled.

LGTM.

LGTM.

Fix a bug & Add Fix me to tests.

Thanks for the reminding! I will follow this rule in future work.

My main point is that the difference between 1) and 3) has to be determined on a case-by-case basis by inspecting the particular expansion sequence, and therefore this should be checked in-line in each expansion sequence. This is why I'd prefer to have each affected custom expansion sequence implementation directly the flag check whether or not strict semantics must be enforced or not; I don't believe this can be a single check just at the top of ExpandNode.

Thanks for the explanation! It very helpful for us to understand the workflow of strict semantics.

Thanks for the detailed guidance. I have merged it to 9.0 branch now. Thank you!

Hi @rnk, I guess we need to merge it to 9.0 branch, right? I haven't did such before, do I need wait for someone approval? Is there someone responsible for committing, or I can do it myself?

In D68857#1743355, @craig.topper wrote:

How about we add a command line option to SelectionDAGISel to disable the mutation code and pass that option on the tests?

This patch is abandoned due to the similar patch D68121 was accepted.

Add the missing attributes #0 = { strictfp }

Rename scalar test file to fp-strict-scalar.

Add test case for scalar instructions.

Separate strict node handling from former patch. This patch only models MXCSR.

Address review comments, add affected tests.

In D68121#1685355, @craig.topper wrote:

Does this really not affect any tests? I would have thought some test would print MIR output that would show the implicit use that wasn't there before.

In D68121#1685679, @RKSimon wrote:

Instead of attributing all these instructions with MXCSR first wouldn't we be better off starting just attributing (V)LDMXCSR/(V)STMXCSR in this patch and then future patches sets up groups of instructions with suitable tests?

The difficult is we may have X87/SSE/AVX/AVX512 instructions under the same node, so I'm planning to mode the attribute of instructions by X87/SSE/AVX/AVX512 first, then set up instructions by node.
And we don't need to model MXCSR for (V)LDMXCSR/(V)STMXCSR, because they are using hasSideEffects and cannot be replaced due to we only modeling rounding and exception bits in MXCSR.

@pengfei , do you think the "strictfp" attribute on an intrinsic would tell you enough for X86 instruction selection? I think for any x86 instructions that take rounding mode operands the intrinsic would already have that explicitly specified, right?

I think the exception attribute on architecture-specific intrinsics should accept option ignore/maytrap/strict like the constrained intrinsics do. "strictfp" is not enough if we want to attach exception attribute to intrinsic.
For rounding mode, AFAIK, some SSE/AVX intrinsics may implicitly use MXCSR.RC, e.g. __m128i _mm_cvtpd_epi32 (__m128d a), AVX512 intrinsics as well as partial SSE/AVX intrinsics can be specified through parameter, but they still may use MAXCSR.RC if specified with _MM_FROUND_CUR_DIRECTION. Should we attach the rounding attribute to intrinsic, if we had explicitly specified MXCSR.RC?