Page MenuHomePhabricator

Document rounding for llvm.lround and llvm.lrint
ClosedPublic

Authored by kpn on Oct 10 2019, 9:34 AM.

Details

Summary

The handling of rounding is not fully documented for the llvm.lround and llvm.lrint family of intrinsics.

This documentation was requested during the reviews of D64746.

Diff Detail

Event Timeline

kpn created this revision.Oct 10 2019, 9:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 10 2019, 9:34 AM

These intrinsics don't raise exceptions, semantically; they're readnone.

kpn updated this revision to Diff 229145.Nov 13 2019, 11:28 AM
kpn retitled this revision from Document rounding and exceptions for llvm.lround and llvm.lrint to Document rounding for llvm.lround and llvm.lrint.
kpn edited the summary of this revision. (Show Details)

Drop the verbiage about exceptions. Update ticket description and title.

craig.topper added inline comments.Nov 13 2019, 12:19 PM
llvm/docs/LangRef.rst
12788–12790

Doesn't lrint use the current rounding mode?

efriedma added inline comments.Nov 13 2019, 12:30 PM
llvm/docs/LangRef.rst
12788–12790

Sort of? I mean, in this context, we assume the "current rounding mode" is round-to-nearest-even.

Probably worth clarifying what we mean by "nearest" here.

kpn marked an inline comment as done.Nov 13 2019, 12:40 PM
kpn added inline comments.
llvm/docs/LangRef.rst
12788–12790

We specify how rounding is done very rarely, and when we do we usually say "nearest integer" without mentioning the rounding mode.

Do we want to change all references to "nearest integer" to state that the current rounding mode is used? Do we want a mention somewhere else to cover all uses?

efriedma added inline comments.Nov 13 2019, 12:55 PM
llvm/docs/LangRef.rst
12788–12790

We only use "nearest integer" in connection to rint/round/etc. In that context, it's not obvious to a casual reader what this means in the context of IEEE arithmetic.

In other contexts, floating-point arithmetic uses the default floating-point rounding mode; this is stated explicitly at http://llvm.org/docs/LangRef.html#floating-point-environment .

kpn marked an inline comment as done.Nov 13 2019, 1:00 PM
kpn added inline comments.
llvm/docs/LangRef.rst
12788–12790

Looking more closely, the *lrint parts of this patch are just grammar changes. I could commit those tomorrow and this patch would be just *lround. The *lround part of the patch is explicit in stating the rounding. Does that seem reasonable?

efriedma accepted this revision.Nov 13 2019, 2:48 PM
efriedma added inline comments.
llvm/docs/LangRef.rst
12788–12790

Fine. In that case, LGTM

This revision is now accepted and ready to land.Nov 13 2019, 2:48 PM
This revision was automatically updated to reflect the committed changes.