This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Fix select_cc lowering for fp16
ClosedPublic

Authored by olista01 on Mar 1 2019, 1:28 AM.

Details

Summary

When lowering a select_cc node where the true and false values are of type f16, we can't use a general conditional move because the FP16 instructions do not support conditional execution. Instead, we must ensure that the condition code is one of the four supported by the VSEL instruction.

Diff Detail

Repository
rL LLVM

Event Timeline

olista01 created this revision.Mar 1 2019, 1:28 AM
SjoerdMeijer added inline comments.Mar 1 2019, 4:07 AM
test/CodeGen/ARM/vsel-fp16.ll
2 ↗(On Diff #188863)

Just for simplicity, can we omit +fp16 here? Or do we actually need it?

For a moment I was wondering if we also need to test this with -fullfp16? I appreciate we are not testing this change anymore, but then remembered we are doing that in fp16-instructions.ll, which also contains some VSEL checks. Perhaps we can move these tests to there, or vice versa, to keep the VSEL tests together?

olista01 marked an inline comment as done.Mar 1 2019, 6:11 AM
olista01 added inline comments.
test/CodeGen/ARM/vsel-fp16.ll
2 ↗(On Diff #188863)

I think I can remove both +fp-armv8 and +fp16.

I don't think we need to test with -fullfp16: in that case f16 isn't a legal type in the backend, so these code paths won't get hit. I copied the tests from vsel.ll and changed the types, because that looks like a comprehensive set of tests for the different condition codes.

SjoerdMeijer accepted this revision.Mar 5 2019, 12:45 AM
SjoerdMeijer added inline comments.
test/CodeGen/ARM/vsel-fp16.ll
2 ↗(On Diff #188863)

sounds good, lgtm.

This revision is now accepted and ready to land.Mar 5 2019, 12:45 AM
This revision was automatically updated to reflect the committed changes.