This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Implement f16 fcopysign and fcopysign(f32, f64)
ClosedPublic

Authored by kzhuravl on Jan 9 2017, 3:29 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

kzhuravl updated this revision to Diff 83722.Jan 9 2017, 3:29 PM
kzhuravl retitled this revision from to [AMDGPU] Implement f16 fcopysign.
kzhuravl updated this object.
kzhuravl added reviewers: arsenm, tstellarAMD.
kzhuravl added subscribers: b-sumner, llvm-commits.
arsenm added inline comments.Jan 9 2017, 3:32 PM
lib/Target/AMDGPU/SIISelLowering.cpp
313–314 ↗(On Diff #83722)

This shouldn't be necessary. This will only be false for R600 targets and the default is expand for illegal types

lib/Target/AMDGPU/SIInstructions.td
680–683 ↗(On Diff #83722)

It is possible to have mismatched FP types for src0 and src1. If you can come up with a testcases combined with the fp casts you might need that too

test/CodeGen/AMDGPU/fcopysign.f16.ll
1 ↗(On Diff #83722)

No -mcpu=SI

kzhuravl updated this revision to Diff 84074.Jan 11 2017, 10:11 PM
kzhuravl edited edge metadata.
kzhuravl marked 3 inline comments as done.

Address review feedback

arsenm added inline comments.Jan 11 2017, 10:13 PM
lib/Target/AMDGPU/SIInstructions.td
688 ↗(On Diff #84074)

This should use e64

696 ↗(On Diff #84074)

e64

708 ↗(On Diff #84074)

e64

test/CodeGen/AMDGPU/fcopysign.f16.ll
1 ↗(On Diff #84074)

This should also have an SI run line. The -enable-unsafe-fp-math should also be dropped

kzhuravl added inline comments.Jan 11 2017, 10:19 PM
test/CodeGen/AMDGPU/fcopysign.f16.ll
1 ↗(On Diff #84074)

Should check lines also be added for SI? Or run line is sufficient?

kzhuravl added inline comments.Jan 11 2017, 10:26 PM
test/CodeGen/AMDGPU/fcopysign.f16.ll
1 ↗(On Diff #84074)

Nevermind. I got confused by the original comment.

kzhuravl updated this revision to Diff 84169.Jan 12 2017, 12:56 PM
kzhuravl retitled this revision from [AMDGPU] Implement f16 fcopysign to [AMDGPU] Implement f16 fcopysign and fcopysign(f32, f64).
kzhuravl edited edge metadata.
kzhuravl marked 3 inline comments as done.

Address review feedback + implement fcopysign(f32, f64)

kzhuravl marked 3 inline comments as done.Jan 12 2017, 12:57 PM
arsenm accepted this revision.Jan 12 2017, 6:48 PM
arsenm edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jan 12 2017, 6:48 PM
This revision was automatically updated to reflect the committed changes.