[PATCH] D69281: [FPEnv] Constrained FCmp intrinsics

Wed Nov 27 12:32:03 PST 2019

craig.topper added a comment.

In D69281#1762150 <https://reviews.llvm.org/D69281#1762150>, @andrew.w.kaylor wrote:

> To clarify, I was suggesting a single intrinsic with quiet/signaling encoded in the predicate. I think the IR definition is a bit cleaner this way and it maps to both the IEEE specification and at least the most common way (I think) this is implemented in x86 hardware and probably others. That said, if doing it this way makes the implementation more complicated I could go along with separate intrinsics and ISD opcodes.

Legacy X87 has separate instructions for signalling and quiet. Originally with FCOM/FUCOM that write to the condition code bits in FPSW. Later with FCOMI and FUCOMI that write to EFLAGS. SSE added similar scalar comparisons that are split between signalling and quiet that produce an EFLAGS result, COMISS/COMISD and UCOMISS/UCOMISD. The flags are defined in a weird way that requires two branches or two setcc instructions and a logic op to implement oeq and une.

SSE also added scalar and vector instructions, CMPPS/CMPD/CMPSS/CMPSD, that produce a alls 0s or all 1s result in each element of a vector register. These instructions have 8 encodings, some that are signalling and some that are quiet. And for some behaviors we have to commute operands. They can't represent all 32 possible behaviors. So for vector comparisons we'll need to scalarize when we don't have the right encoding. With AVX the 8 encodings where extended to 32 encodings that support all possible behaviors.

Having two different ISD opcodes is straightforward for FCOM/FCOMI/FUCOMI/FCOMI/COMISS/COMISD/UCOMISS/UCOMISD selection. A single ISD opcode with merged predicate would also be straightforward.

The CMPPS/CMPD/CMPSS/CMPSD selection in X86 is already in custom code due to the 8 encoding issue. So two ISD opcodes or one probably doesn't make a lot of difference there either.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69281/new/

https://reviews.llvm.org/D69281