This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Restrict ieee_mode to HSA.
AbandonedPublic

Authored by jvesely on Nov 27 2017, 12:23 PM.

Details

Reviewers
arsenm
b-sumner
Summary

ieee_mode specificaly governs behaviour of SNaNs.
OpenCL does not want this, OpenGL does not care.
Let's just disable it for everything but HSA compute.

Diff Detail

Repository
rL LLVM

Event Timeline

jvesely created this revision.Nov 27 2017, 12:23 PM
arsenm edited edge metadata.EditedNov 27 2017, 12:35 PM

I think if we're going to do this we should just never use IEEE mode even for HSA. I don't think it buys us anything we want. This probably needs more discussion though

I think if we're going to do this we should just never use IEEE mode even for HSA. I don't think it buys us anything we want. This probably needs more discussion though

That'd need someone form the HSA team to chime in. Do you know of anyone specific to add as reviewer/subscriber?

t-tye added a comment.Nov 27 2017, 9:19 PM

I think if we're going to do this we should just never use IEEE mode even for HSA. I don't think it buys us anything we want. This probably needs more discussion though

That'd need someone form the HSA team to chime in. Do you know of anyone specific to add as reviewer/subscriber?

HSA does define support for IEEE exception tracking. I do not believe we have surfaced that in the LLVM compiler, nor have any languages using the ability. So we may be able to disable. Adding @b-sumner as a reviewer to advise.

b-sumner edited edge metadata.Nov 28 2017, 6:13 AM

IEEE mode disables output modifiers, which is good since output modifiers are not IEEE compatible and do not support output subnormal values.

I suppose if we have some other mechanism to ensure output modifiers are never used, then we could consider running with IEEE=0, but I think we need to continue running with IEEE=1 for compute. There may be users counting on current behavior.

jvesely added a comment.EditedNov 28 2017, 12:37 PM

IEEE mode disables output modifiers, which is good since output modifiers are not IEEE compatible and do not support output subnormal values.

that should be enforceable in codegen.

I suppose if we have some other mechanism to ensure output modifiers are never used, then we could consider running with IEEE=0, but I think we need to continue running with IEEE=1 for compute. There may be users counting on current behavior.

The current behavior is broken for OpenCL so no one should depend on it.
This patch preserves behaviour for HSA.

I suppose if we have some other mechanism to ensure output modifiers are never used, then we could consider running with IEEE=0, but I think we need to continue running with IEEE=1 for compute. There may be users counting on current behavior.

The current behavior is broken for OpenCL so no one should depend on it.

What is broken about the current behavior?

jvesely added a comment.EditedNov 28 2017, 7:05 PM

The current behavior is broken for OpenCL so no one should depend on it.

What is broken about the current behavior?

"fmin and fmax behave as defined by C99 and may not match the IEEE 754-2008 definition for minNum and
maxNum with regard to signaling NaNs. Specifically, signaling NaNs may behave as quiet NaNs." -- OpenCL 1.2 specs

the CL CTS checks this behaviour, so we either need to flush SNaNs before calling v_min/v_max or disable the ieee_mode for CL. this patch does the latter.
as for GL compute I'd expect the behaviour to follow that of graphics GLSL, which is currently not the case

AFAIK this just enables sNaN handling (which won't work correctly anyway). The cost is is extra canonicalize instructions for min/max, and then prevents us from using omod when denormals are disabled.

jvesely updated this revision to Diff 131723.Jan 28 2018, 12:21 PM

rebased.

If HSA wants to keep the broken behaviour let them keep it. Don't force the breakage on other environments.

jvesely abandoned this revision.Jun 7 2018, 2:23 PM