This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Add default denormal mode to MachineFunctionInfo
ClosedPublic

Authored by arsenm on Oct 31 2019, 6:52 PM.

Details

Summary

The default FP mode should really be a property of a specific
function, and not a subtarget. Introduce the necessary fields to the
SIMachineFunctionInfo to help move towards this goal.

Diff Detail

Event Timeline

arsenm created this revision.Oct 31 2019, 6:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 31 2019, 6:52 PM

As far as I understand this changes the default. Currently default depends on the ASIC and its ability to support denorms fast. I think we want to keep it this way.

As far as I understand this changes the default. Currently default depends on the ASIC and its ability to support denorms fast. I think we want to keep it this way.

This is a decision for the frontend. Undecorated IR functions need to be the conservatively correct default. Currently clang puts the subtarget feature on functions (except in the device library build). Eventually that will be replaced with a function attribute. Currently with the default of no denormals set by the backend, the library is miscompiled whenever it's not inlined

As far as I understand this changes the default. Currently default depends on the ASIC and its ability to support denorms fast. I think we want to keep it this way.

This is a decision for the frontend. Undecorated IR functions need to be the conservatively correct default. Currently clang puts the subtarget feature on functions (except in the device library build). Eventually that will be replaced with a function attribute. Currently with the default of no denormals set by the backend, the library is miscompiled whenever it's not inlined

This also doesn't change the default, this is an infrastructure fix to move towards fixing the IR default

rampitec accepted this revision.Oct 31 2019, 10:04 PM

Since default is the same LGTM. Before switching we need to make sure clang correctly sets the default and check consequences for the library.

This revision is now accepted and ready to land.Oct 31 2019, 10:04 PM