This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Implement llvm.set.rounding intrinsic
AbandonedPublic

Authored by qiucf on Jul 11 2023, 2:22 AM.

Details

Reviewers
nemanjai
shchenz
sepavloff
Group Reviewers
Restricted Project
Summary

According to LangRef, llvm.set.rounding sets rounding mode by integer argument:

0 - toward zero
1 - to nearest, ties to even
2 - toward positive infinity
3 - toward negative infinity
4 - to nearest, ties away from zero

While PowerPC ISA says:

0 - to nearest
1 - toward zero
2 - toward positive infinity
3 - toward negative infinity

This patch maps the argument and write into last two bits of FPSCR (rounding mode).

Diff Detail

Event Timeline

qiucf created this revision.Jul 11 2023, 2:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2023, 2:22 AM
qiucf requested review of this revision.Jul 11 2023, 2:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2023, 2:22 AM

What is the plan for handling nearest, away rounding mode for which the PPC FPSCR does not have a setting?

llvm/lib/Target/PowerPC/PPCISelLowering.cpp
8854

Why not produce much simpler code when the parameter is constant (using mtfsb[01])?

11834

Please refrain from unrelated formatting changes.

nemanjai added inline comments.Jul 11 2023, 4:42 AM
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
8875

Is this whole complicated sequence actually better than just expanding this into a library call?

qiucf updated this revision to Diff 542074.Jul 19 2023, 9:29 AM
qiucf marked 2 inline comments as done.

Use mtfsb0/1 for constant cases.

qiucf added a comment.Jul 19 2023, 9:30 AM

What is the plan for handling nearest, away rounding mode for which the PPC FPSCR does not have a setting?

According to other arch's implementation, if input is constant, do assert when the mode is unsupported; if it's variable, only handle the lower 2-bits.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp
8875

The glibc fesetround routine writes as:

if (mode > 2) {
  mtfsb1(30);
  if (mode == 3) mtfsb1(31); else mtfsb0(31);
} else {
  mtfsb0(30);
  if (mode == 1) mtfsb1(31); else mtfsb0(31);
}

This indeed looks better. But it seems it's not convenient to generate multiple branches in DAG legalization, also for a library call not recorded.

I think such intrinsic should not have any assumption on runtime library (since fesetround is included in libc, not compiler-rt).