This is an archive of the discontinued LLVM Phabricator instance.

[FEnv] Small fixes to implementation of flt.rounds
ClosedPublic

Authored by sepavloff on May 4 2020, 4:52 AM.

Details

Summary

This change makes assorted correction related to the intrinsic
llvm.flt.rounds:

  • Added documentation entry in LangRef,
  • Attributes of the intrinsic changed to be in line with other functions dependent of floating-point environment,

Diff Detail

Event Timeline

sepavloff created this revision.May 4 2020, 4:52 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 4 2020, 4:52 AM
sepavloff updated this revision to Diff 264184.May 15 2020, 2:06 AM

Updated patch

  • rebased,
  • corrected documentation,
  • fixed formatting.
lenary added inline comments.May 15 2020, 4:19 AM
llvm/test/CodeGen/RISCV/flt-rounds.ll
23

I'm interested to understand how this function is provided. Is it part of compiler-rt or libgcc, or is it provided another way?

sepavloff marked an inline comment as done.May 15 2020, 9:17 AM
sepavloff added inline comments.
llvm/test/CodeGen/RISCV/flt-rounds.ll
23

At least musl library provides its implementation: https://git.musl-libc.org/cgit/musl/tree/src/fenv/__flt_rounds.c . Similar implementation exists in glibc.

craig.topper added inline comments.May 16 2020, 2:57 PM
llvm/test/CodeGen/RISCV/flt-rounds.ll
23

A quick google search for "glibc __flt_rounds" only shows it in a powerpc nofpu file.

sepavloff updated this revision to Diff 264623.May 18 2020, 7:49 AM

Updated patch

Default lowering of FLT_ROUNDS_ again is made by replacing the node with
constant 1, which denotes rounding to nearest, ties to even. It is incorrect,
but is compatible with with current behavior. Using more correct lowering to
function call may cause problems for targents that rely on default lowering,
because not all C standard libraries implements __flt_rounds.

sepavloff marked an inline comment as done.May 18 2020, 8:00 AM
sepavloff added inline comments.
llvm/test/CodeGen/RISCV/flt-rounds.ll
23

GCC defined FLT_ROUNDS in float.h as 1, which is incorrect in general case. As for __builtin_flt_rounds, now GCC does not have such intrinsic. So using FLT_ROUND on some targets might be tied to this kludge and lowering it to function call can cause fails. So lowering to a constant was restored.

sepavloff edited the summary of this revision. (Show Details)May 18 2020, 8:00 AM
sepavloff updated this revision to Diff 265990.May 25 2020, 3:03 AM
sepavloff edited the summary of this revision. (Show Details)

Updated patch

  • Removed support for lowering to library call at all. Now it is NFC patch.
This revision is now accepted and ready to land.May 25 2020, 11:56 AM
This revision was automatically updated to reflect the committed changes.