[PATCH] D77013: [AMDGPU] Add options -mamdgpu-ieee -mno-amdgpu-ieee

Tue Apr 20 10:10:50 PDT 2021

arsenm added a comment.

In D77013#2702129 <https://reviews.llvm.org/D77013#2702129>, @yaxunl wrote:

> The recent change https://reviews.llvm.org/D96280 caused some difficulty for this patch. I would like to have some suggestions.
>
> Basically current FE requires any codegen or target option should be uniquely reproduced from its internal representation. The current patch does not satisfy the requirement. That's why it fails the pre-merge check.
>
> Currently, the internal representation CodeGenOpt.EmitIEEENaNCompliantInsts is not uniquely determined by the command line options -m[no]-amdgpu-ieee. It is also determined by LangOptsRef.NoHonorNaNs. Therefore I cannot regenerate the original command line option by the value of CodeGenOpt.EmitIEEENaNCompliantInsts.

These are two independent options. -mno-amdgpu-ieee should require no-nans-fp-math to be used by the driver. The -m flag is an ABI changing option, and we don't implement the semantics required to make it work without knowing there are no signaling nans

> One solution I can think of, is to use -mamdgpu-ieee={default|on|off}. This way I should be able to reproduce the original command line.

The default should only be on. Why is this not a problem for every other -m/-mno flag?

================
Comment at: clang/include/clang/Basic/CodeGenOptions.def:428-429

+/// Floating point opcodes that support exception flag gathering quiet and
+/// propagate signaling NaN inputs per IEEE 754-2008 (AMDGPU Only)
+CODEGENOPT(EmitIEEENaNCompliantInsts, 1, 1)
----------------
This should explain this is setting a bit in the expected default floating point mode register

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77013/new/

https://reviews.llvm.org/D77013