This is an archive of the discontinued LLVM Phabricator instance.

[X86][F16C] Remove cvtph2ps intrinsics and use generic half2float conversion (PR37554)
ClosedPublic

Authored by RKSimon on Feb 26 2020, 4:01 AM.

Details

Summary

This removes everything but int_x86_avx512_mask_vcvtph2ps_512 which provides the SAE variant, but even this can use the fpext generic if the rounding control is the default.

I have a new f16c-intrinsics-upgrade.ll file as well but for some reason git diff is going weird when I try to include it, basically all the ph2ps tests are cut+paste from f16c-intrinsics.ll

Diff Detail

Unit TestsFailed

Event Timeline

RKSimon created this revision.Feb 26 2020, 4:01 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFeb 26 2020, 4:01 AM

Should we check that this generates constrained.fpext in strict mode?

Should we check that this generates constrained.fpext in strict mode?

Sure - where is the best place(s) to test this?

Should we check that this generates constrained.fpext in strict mode?

Sure - where is the best place(s) to test this?

For the intrinsics we've tested this way so far, they're in sse-builtins-constrained.c and similar. I think we have separate files for compares that I should merge into the others. They got commited separately as I was working on compares when someone else was working on the others so I made a separate file to avoid the merge conflict.

I've posted https://reviews.llvm.org/D75304 to make the backend recognize the strict version of this pattern.

RKSimon updated this revision to Diff 247288.Feb 28 2020, 9:15 AM

Add strict fp codegen tests - this means the patch is now dependent on D75304

This revision is now accepted and ready to land.Feb 28 2020, 10:26 AM
This revision was automatically updated to reflect the committed changes.
hoyFB added a subscriber: hoyFB.Mar 3 2020, 6:37 PM
hoyFB added inline comments.
llvm/include/llvm/IR/IntrinsicsX86.td
2551

Hello, we have some tools generating these intrinsics being removed here. Can you suggest what we should generate on LLVM IR with this change?

craig.topper added inline comments.Mar 3 2020, 8:41 PM
llvm/include/llvm/IR/IntrinsicsX86.td
2551

I think the diffs to test/CodeGen/f16c-builtins.c should show the difference. You can also follow the code in the changes to AutoUpgrade.cpp