This is an archive of the discontinued LLVM Phabricator instance.

[FPEnv] Remove unnecessary rounding mode argument for constrained intrinsics
ClosedPublic

Authored by uweigand on Dec 9 2019, 11:10 AM.

Details

Summary

The following intrinsics currently carry a rounding mode metadata argument:

  • llvm.experimental.constrained.minnum
  • llvm.experimental.constrained.maxnum
  • llvm.experimental.constrained.ceil
  • llvm.experimental.constrained.floor
  • llvm.experimental.constrained.round
  • llvm.experimental.constrained.trunc

This is not useful since the semantics of those intrinsics do not in any way depend on the rounding mode. In similar cases, other constrained intrinsics do not have the rounding mode argument. Remove it here as well.

Diff Detail

Event Timeline

uweigand created this revision.Dec 9 2019, 11:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 9 2019, 11:10 AM
kpn added a comment.Dec 9 2019, 11:15 AM

I think this is a good idea.

Random thought: Do we need to document which version of 754 this conforms to?

I've also been wondering if we should drop rounding from ceil, floor, trunc, and round. Looks like lround/llround don't have rounding.

In D71218#1775689, @kpn wrote:

Random thought: Do we need to document which version of 754 this conforms to?

This seems to be a much more complicated topic than I expected, there are different versions of IEEE 754 and different versions of the C standard and different implementations in various libc versions, and none of them completely agree with each other ... In any case, to avoid duplication we probably should only say the the constrained version follows the same semantics as the "regular" LLVM intrinsics.

I've also been wondering if we should drop rounding from ceil, floor, trunc, and round. Looks like lround/llround don't have rounding.

Yes, I think it is probably best to only have the rounding mode argument for those functions that actually depend on the current rounding mode. I can remove it from those four as well.

uweigand updated this revision to Diff 233047.Dec 10 2019, 4:19 AM
uweigand retitled this revision from [FPEnv] Remove rounding mode argument for constrained minnum/maxnum to [FPEnv] Remove unnecessary rounding mode argument for constrained intrinsics.
uweigand edited the summary of this revision. (Show Details)
uweigand updated this revision to Diff 233624.Dec 12 2019, 8:27 AM

Updated to accommodate upstream changes in clang and the IRBuilder.

This revision is now accepted and ready to land.Dec 17 2019, 10:05 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptDec 17 2019, 12:12 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript