This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Add support for f16 builtin functions for VI+
ClosedPublic

Authored by kzhuravl on Nov 9 2016, 3:08 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

kzhuravl updated this revision to Diff 77402.Nov 9 2016, 3:08 PM
kzhuravl retitled this revision from to [AMDGPU] Add support for f16 builtin functions for VI+.
kzhuravl updated this object.
kzhuravl added reviewers: tstellarAMD, arsenm.
kzhuravl added a subscriber: cfe-commits.
arsenm added inline comments.Nov 11 2016, 10:03 AM
test/CodeGenOpenCL/builtins-amdgcn-error-f16-class.cl
1–9 ↗(On Diff #77402)

These tests can all be in the same file

kzhuravl added inline comments.Nov 11 2016, 10:08 AM
test/CodeGenOpenCL/builtins-amdgcn-error-f16-class.cl
1–9 ↗(On Diff #77402)

The problem is if I put them into the same file only the first function gives the error '__builtin_amdgcn_*h' needs target feature 16-bit-insts and I did not have time to investigate why, but I was planning to do it once I finish something else.

Would leaving them in the separate files be acceptable short-term?

arsenm added inline comments.Nov 11 2016, 10:10 AM
test/CodeGenOpenCL/builtins-amdgcn-error-f16-class.cl
1–9 ↗(On Diff #77402)

I think putting all of the half instructions in one file is fine with an f16-less target, you don't need one for each individual error.

Also target-cpu tahiti + amdhsa doesn't make sense, one or the other should be changed

yaxunl added inline comments.Nov 11 2016, 10:24 AM
test/CodeGenOpenCL/builtins-amdgcn-error-f16-div-fixup.cl
8 ↗(On Diff #77402)

The tests for checking error messages are usually put in SemaOpenCL.

kzhuravl updated this revision to Diff 77633.Nov 11 2016, 10:36 AM
kzhuravl edited edge metadata.
kzhuravl marked 4 inline comments as done.

Address review feedback: put tests in the same file, update run line, move error tests to SemaOpenCL directory

kzhuravl updated this revision to Diff 77634.Nov 11 2016, 10:39 AM
kzhuravl edited edge metadata.

Also update run line to exclude amdhsa from another error file.

kzhuravl updated this revision to Diff 77654.Nov 11 2016, 12:36 PM
kzhuravl edited edge metadata.

Leave the return type of frexp_exph unchanged

arsenm accepted this revision.Nov 11 2016, 1:42 PM
arsenm edited edge metadata.

LGTM

This revision is now accepted and ready to land.Nov 11 2016, 1:42 PM
This revision was automatically updated to reflect the committed changes.
cfe/trunk/lib/CodeGen/CGBuiltin.cpp