Page MenuHomePhabricator

[x86] Adding support for some missing intrinsics: _castf32_u32, _castf64_u64, _castu32_f32, _castu64_f64
ClosedPublic

Authored by yubing on Sep 5 2019, 12:11 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

yubing created this revision.Sep 5 2019, 12:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 5 2019, 12:11 AM
RKSimon added inline comments.Sep 5 2019, 1:39 AM
clang/lib/Headers/ia32intrin.h
198 ↗(On Diff #218845)

There's an unofficial agreement that new x86 intrinsics should have doxygen comments - someday we'll get around to adding them to the existing ones as well.....

207 ↗(On Diff #218845)

Shouldn't the unsigned long long cases be hidden by x86_64 wrappers?

clang/test/CodeGen/miscellaneous-builtins.c
1 ↗(On Diff #218845)

32-bit target tests?

32 ↗(On Diff #218845)

newline please

Also, as its x86 specific miscellaneous-builtins.c should be called x86-builtins.c (or similar).

craig.topper added inline comments.Sep 5 2019, 2:43 PM
clang/lib/Headers/ia32intrin.h
207 ↗(On Diff #218845)

icc doesn't restrict this to 64-bit targets. On 64-bit targets it corresponds to movq, but on other targets we can just lower to a 64-bit store and two 32-bit loads in the worst case.

RKSimon added inline comments.Sep 6 2019, 1:45 AM
clang/lib/Headers/ia32intrin.h
207 ↗(On Diff #218845)

Thanks - in which case we should definitely add 32-bit triple tests

yubing updated this revision to Diff 221250.Sep 22 2019, 8:36 PM
yubing marked 3 inline comments as done.Sep 22 2019, 10:18 PM

Please rename miscellaneous-builtins.c to x86-builtins.c

yubing updated this revision to Diff 221473.Sep 23 2019, 11:56 PM
RKSimon accepted this revision.Sep 24 2019, 5:54 AM

LGTM

This revision is now accepted and ready to land.Sep 24 2019, 5:54 AM
This revision was automatically updated to reflect the committed changes.