[PATCH] D64746: Add constrained intrinsics for lrint and lround

[Kevin P. Neal via Phabricator via llvm-commits]

kpn added a comment.

In D64746#3325655 <https://reviews.llvm.org/D64746#3325655>, @nlopes wrote:

> In D64746#3325599 <https://reviews.llvm.org/D64746#3325599>, @uweigand wrote:
>
>> In D64746#3324418 <https://reviews.llvm.org/D64746#3324418>, @nlopes wrote:
>>
>>> In D64746#3324270 <https://reviews.llvm.org/D64746#3324270>, @uweigand wrote:
>>>
>>>> This matches my understanding.   (Though I must admit I'm not 100% confident I fully understand the precise distinction between "undefined behavior", a poison value, and an undef value ...  It's certainly one of those :-)  But from my reading of the IR reference, "poison" does indeed look correct here.)
>>>
>>> OK, thank you!
>>> It's poison. UB would be too strong. I've put on my todo list to fix LangRef as well.
>>
>> The reason why I was hesitating about poison vs. UB is exceptions.   This is clearer when talking about the exception metadata as opposed to the rounding mode metadata:  if exception metadata promises that FP exceptions are disabled, but they are actually enabled at runtime, compiler optimizations are free to generate code triggering spurious exceptions that might kill the program outright - that seems more like UB than poison to me.
>>
>> With the rounding mode, this is much less obvious to me, but I guess in theory it could happen that if the actual rounding mode does not match the mode promised by the metadata, a compiler optimization might possibly generate code that could now e.g. introduce an underflow where the unoptimized code would not have one, and if in addition exception are also enabled, that extra underflow could now trigger an extra exception.   However, I'm not sure if that can actually ever occur in practice ...
>
> Thank you!
> Makes sense. I need to think a bit more about this.
> Ideally we want to allow intrinsics marked as not throwing exceptions to be executed speculatively. The semantics needs to be crafted to allow that scenario.
> I think returning poison for the rounding mode is OK as that is sufficient to trigger an exception (as poison can be replaced with any value). So we can keep the mismatch in rounding modes as poison. And then move the complexity to the exception semantics. It's not obvious to me what's the ideal semantics yet.

I'm a little worried about the runtime rounding mode being changed and speculative execution moving an instruction to a point where the wrong rounding mode is in effect. I think a dynamic rounding mode intrinsic call shouldn't be moved past a function call. I'm pretty sure the PowerPC backend already has this rule. But it's still possible for a function call to change the rounding mode with the rounding mode metadata properly representing this, but code movement optimizations might not take this into account and we'll have a problem.

I'm pretty sure the rounding metadata is there because the compiler can't know the actual rounding mode used at runtime. So I don't know how the compiler can know about UB and I don't know how it can use undef or poison.


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

https://reviews.llvm.org/D64746