This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Armv8.2-A FP16 code generation (part 3/3)
ClosedPublic

Authored by SjoerdMeijer on Feb 2 2018, 9:04 AM.

Details

Summary

This adds most of the FP16 codegen support, but these areas need further work:

  • FP16 literals and immediates are not properly supported yet (e.g. literal pool needs work),
  • instructions that are generated from intrinsics (e.g. vabs) haven't been added.

This will be addressed in follow-up patches.

Diff Detail

Repository
rL LLVM

Event Timeline

SjoerdMeijer created this revision.Feb 2 2018, 9:04 AM
SjoerdMeijer edited the summary of this revision. (Show Details)

Added T32 tests.

olista01 accepted this revision.Feb 5 2018, 2:59 AM

LGTM with a few nits (see inline comments), but please give 24 hours for other time zones to comment before committing.

lib/Target/ARM/ARMInstrVFP.td
531 ↗(On Diff #132793)

The AHuI class already has the HasFullFP16 predicate, so is this needed?

(same question many more times in this file)

784 ↗(On Diff #132793)

Output should be SPR?

1383 ↗(On Diff #132793)

Should this copy to the SPR regclass, since the input is 32-bit?

test/CodeGen/ARM/fp16-instructions.ll
99 ↗(On Diff #132793)

It might be better to write these tests as loading/storing the fp16 values, rather than doing all of this bit-casting, to simplify the tests. However, if you're going to do native support for fp16 args/returns in the backend soon, I'd be fine with committing these as-is for now knowing that they'll be simplified even further by that.

This revision is now accepted and ready to land.Feb 5 2018, 2:59 AM

Thanks for the review!
Nits addressed, so this is the diff I will commit tomorrow.

It might be better to write these tests as loading/storing the fp16 values,
rather than doing all of this bit-casting, to simplify the tests. However, if
you're going to do native support for fp16 args/returns in the backend soon,
I'd be fine with committing these as-is for now knowing that they'll be
simplified even further by that.

Yes, that was exactly my plan: when we have proper fp16 args/return support, I
will update these tests to reflect that. For now I would like to keep the IR
and tests like they are at the moment, because that's the IR that we currently
generate from source code.

This revision was automatically updated to reflect the committed changes.