[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.

In https://reviews.llvm.org/D45616#1290897, @nastafev wrote:

> >   can trigger arbitrary floating-point exceptions anywhere in your code
>
> I believe this statement reflects the current state of many compilers on the market, I guess I just don't see the reason why making things worse. It seems the original intent of the commit was to add support for masked compares, and that could have been achieved without breaking what already worked.
>
> I hope the patch is ultimately helping some performance optimization, but it is breaking the original intent of some legitimate programs that worked before, and introduces correctness regression. So to me it must be at least guarded by a flip-switch.
>
> The reference to constrained floating-point intrinsics work is relevant, but it will obviously take time and extra effort to enable and then to unbreak all the cases that are made broken here. Instead one could postpone lowering of the particular instructions until it was possible without violation of the semantics...


That seems like a legitimate concern to me.

I believe this patch was part of a larger effort to get rid of the dependence on intrinsics. We have a very broad preference for expressing things using native IR whenever possible because (among other reasons) intrinsics block most optimizations and we don't want to teach optimizations to reason about target-specific intrinsics. In this particular case we may have overreached because it isn't strictly possible to express all the semantics of this built-in accurately using native IR.

There is a patch currently active to add constrained fcmp intrinsics, but it doesn't have a masked variant.


Repository:
  rC Clang

https://reviews.llvm.org/D45616