This is an archive of the discontinued LLVM Phabricator instance.

SelectionDAG: Support Expand of f16 extloads
ClosedPublic

Authored by arsenm on Jul 9 2015, 1:06 PM.

Details

Reviewers
ab
Summary

Currently this hits an assert that extload should
always be supported, which assumes integer extloads.

This moves a hack out of SI's argument lowering and
is covered by existing tests.

Diff Detail

Event Timeline

arsenm updated this revision to Diff 29378.Jul 9 2015, 1:06 PM
arsenm retitled this revision from to SelectionDAG: Support Expand of f16 extloads .
arsenm updated this object.
arsenm added a subscriber: llvm-commits.
ab accepted this revision.Aug 24 2015, 2:15 PM
ab added a reviewer: ab.
ab added a subscriber: ab.

I'll admit I'm not sure I understand what the test does (%arg is loaded?), but the code seems reasonable.

lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
876–881

Should this be part of EVT, with a more robust extended handling? Say, "changeTypeToInteger".

1120

"use allow" -> "allow using" ?

test/CodeGen/AMDGPU/half.ll
125–137

I assume these functions might go through the same codepath, can you add checks for them?

This revision is now accepted and ready to land.Aug 24 2015, 2:15 PM
arsenm updated this revision to Diff 33018.Aug 24 2015, 4:15 PM
arsenm edited edge metadata.

Move toIntegerVT to EVT

arsenm marked an inline comment as done.Aug 24 2015, 4:16 PM
In D11081#231467, @ab wrote:

I'll admit I'm not sure I understand what the test does (%arg is loaded?), but the code seems reasonable.

Yes, kernel arguments are lowered as loads

test/CodeGen/AMDGPU/half.ll
125–137

These are slightly different because the vector arguments aren't promoted like half args are to f32, but I've added checks for them

ab added a comment.Sep 8 2015, 5:04 PM

LGTM, with some documentation for EVT::changeTypeToInteger.

Thanks!

include/llvm/CodeGen/ValueTypes.h
92

A short comment would be appreciated!

arsenm closed this revision.Sep 8 2015, 6:13 PM

r247113