This is an archive of the discontinued LLVM Phabricator instance.

[LegalizeTypes][X86] Add a new strategy for type legalizing f16 type that softens it to i16, but promotes to f32 around arithmetic ops.
ClosedPublic

Authored by craig.topper on Jan 30 2020, 3:24 PM.

Details

Summary

This is based on this llvm-dev thread http://lists.llvm.org/pipermail/llvm-dev/2019-December/137521.html

The current strategy for f16 is to promote type to float every except where the specific width is required like loads, stores, and bitcasts. This results in rounding occurring in odd places instead of immediately after arithmetic operations. This interacts in weird ways with the __fp16 type in clang which is a storage only type where arithmetic is always promoted to float. InstCombine can remove some fpext/fptruncs around such arithmetic and turn it into arithmetic on half. This wouldn't be so bad if SelectionDAG was able to put those fpext/fpround back in when it promotes.

It is also not obvious how to handle to make the existing strategy work with STRICT fp. We need to use STRICT versions of the conversions which require chain operands. But if the conversions are created for a bitcast, there is no place to get an appropriate chain from.

This patch implements a different strategy where conversions are emitted directly around arithmetic operations. And otherwise its passed around as an i16 including in arguments and return values. This can result in more conversions between arithmetic operations, but is closer to matching the IR the frontend generates for __fp16. And it will allow us to use the chain from constrained arithmetic nodes to link the STRICT_FP_TO_FP16/STRICT_FP16_TO_FP that will need to be added. I've set it up so that each target can opt into the new behavior. Converting all the targets myself was more than I was able to handle.

Diff Detail

Event Timeline

craig.topper created this revision.Jan 30 2020, 3:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 30 2020, 3:24 PM

This makes sense, I think.

llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
2685

Using getIntegerVT like this is suspicious; if the original node is a vector, the promoted node should also be a vector, no?

efriedma added inline comments.Jan 30 2020, 4:14 PM
llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
2685

Wait, nevermind; the original node can't be a vector. We would split/scalarize the vector first.

In that case, you might as well just write MVT::i16 explicitly, though.

2766

Is SoftPromoteHalfOp_INSERT_VECTOR_ELT actually reachable? If the operand is a half-float, the result would have to be a half vector, and we would split that, I think?

craig.topper marked an inline comment as done.Jan 30 2020, 8:55 PM
craig.topper added inline comments.
llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
2766

You're right. I'm not sure why I put that in there. I'll remove.

Address review comments so far

efriedma accepted this revision.Jan 31 2020, 10:19 AM

LGTM

llvm/test/CodeGen/X86/vector-half-conversions.ll
512

Sort of a side-note, but it might make sense to make half "legal" on targets with vcvtph2ps .

This revision is now accepted and ready to land.Jan 31 2020, 10:19 AM
This revision was automatically updated to reflect the committed changes.