[X86][LegalizeDAG] FPOWI: promote f16 operand

[v01dXYZ]

Fixes #105747

[llvmbot]

@llvm/pr-subscribers-backend-x86

Author: None (v01dXYZ)

Changes

Fixes #105747

---

Full diff: https://github.com/llvm/llvm-project/pull/105775.diff

1 Files Affected:

• (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+1)

diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index e0face6b6a7904..83bb2577777864 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -614,6 +614,7 @@ X86TargetLowering::X86TargetLowering(const X86TargetMachine &TM,
     setOperationAction(ISD::FTAN, VT, Action);
     setOperationAction(ISD::FSQRT, VT, Action);
     setOperationAction(ISD::FPOW, VT, Action);
+    setOperationAction(ISD::FPOWI, VT, Action);
     setOperationAction(ISD::FLOG, VT, Action);
     setOperationAction(ISD::FLOG2, VT, Action);
     setOperationAction(ISD::FLOG10, VT, Action);

[phoebewang]

Add a test case for it?

[v01dXYZ]

@phoebewang Hi, I hesitate between putting this test in CodeGen/Generic or in CodeGen/X86 b/c some other targets may show the same problem.

I'll put it in CodeGen/X86 as this one won't break the buildbot builds.

[phoebewang]

CodeGen/X86 since you changed the X86 backend code rather than generic code. Besides, some targets may have the test already.

[v01dXYZ]

I discovered something strange with FEXPR/f16. It expands it (when looking at the code it seems rather it converts it to a same sized integer type) instead of promoting it.

BTW the reason I'm looking into that is because that's another Opcode with a default value set to Expand in TargetLoweringBase.cpp and not mentionned in X86ISelLowering.cpp.

[v01dXYZ]

There is something I don't understand in TargetLoweringBase, some default operation actions are Expand but what does it mean to Expand with types that are already the smallest ones possible (in our case, that's f16).

[RKSimon]

Add a test case (probably to avx512fp16-scalar.ll ?)

[RKSimon]

Add a test case (probably to avx512fp16-scalar.ll ?)

I've added fp16-libcalls.ll which should give us better test coverage - please add the powi tests there

[v01dXYZ]

I was worried than another Opcode presented the same problem (ie with LegalizeAction defaulting to Expand and spawning an assert when LegalizeDAG fallthroughs to libcall convertion). But it seems FPOWI is special, that's the only float Opcode in ConvertNodeToLibcall that checks for the libcall existence and if it does not exist fallbacks to using FPOW.

I have a vocabulary question: in the strict terminology of Legalisation, expand means breaking up a complex operation using smaller types, should I rather think of it as an Instruction Expand ?

[RKSimon]

I have a vocabulary question: in the strict terminology of Legalisation, expand means breaking up a complex operation using smaller types, should I rather think of it as an Instruction Expand ?

It depends on what legalization stage you're dealing with - expand to legal type / scalarize / split into simpler instructions etc.

I've added more tests to fp16-libcalls.ll - please can you rebase?

[RKSimon]

LGTM