[PATCH] D45616: [X86] Lower _mm[256|512]_cmp[.]_mask intrinsics to native llvm IR

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added a comment.

Agreed. Reverting this patch wouldn't move us forward on constrained FP handling. What I'm saying (and what I think @nastafev  is saying) is that this patch is taking a built-in that allows the user to express specific signalling behavior and throwing away that aspect of its semantics. Granted we do say that FP environment access is unsupported, so technically unmasking FP exceptions or even reading the status flag is undefined behavior. But it seems like there's a pretty big difference between using that claim to justify enabling some optimization that might do constant folding in a way that assumes the default rounding mode versus using that claim to disregard part of the semantics of a built-in that is modeling a target-specific instruction.

I'm not insisting that we have to revert this patch or anything like that. I'm just saying that we should think about it. My personal preference would actually be to have this code re-implemented using the new constrained fcmp intrinsic when it lands. That still leaves the masking part of this unsettled, but as you say that's probably not a priority right now.

BTW, here's a link to the constrained fcmp review: https://reviews.llvm.org/D54121


Repository:
  rC Clang

https://reviews.llvm.org/D45616