This is an archive of the discontinued LLVM Phabricator instance.

[X86] AVX512FP16 instructions enabling 4/6
ClosedPublic

Authored by pengfei on Jul 1 2021, 12:00 AM.

Diff Detail

Event Timeline

pengfei created this revision.Jul 1 2021, 12:00 AM
pengfei requested review of this revision.Jul 1 2021, 12:00 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 1 2021, 12:00 AM
pengfei edited the summary of this revision. (Show Details)Jul 1 2021, 1:00 AM
pengfei updated this revision to Diff 366473.Aug 14 2021, 9:15 PM

Rebased.
Add extra parentheses for macro.

LuoYuanke added inline comments.Aug 17 2021, 7:24 AM
clang/include/clang/Basic/BuiltinsX86.def
1897

The naming convention is not consistent. Rename it to rndscaleph128?

1898

Ditto.

1899

rndscaleph512?

1906

The name convention is not consistent for scalar version. getexpsh?

pengfei added inline comments.Aug 17 2021, 11:30 PM
clang/include/clang/Basic/BuiltinsX86.def
1897

I agree it should be better to be consistent with the naming convention mostly used. But I found rndscaleps/pd are using the same way here. Changing here will result in inconsistent somewhere.
I think consistent with ps/pd should be better.
The following builtin names have the same problem.

LuoYuanke added inline comments.Aug 18 2021, 6:44 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
1929

Does this node means "round to int"? What's the difference to "FNEARBYINT"?

llvm/lib/Target/X86/X86ISelLowering.h
290 ↗(On Diff #366473)

Move the code to line 283, so that it is adjacent to FRSQRT and FRCP?

llvm/lib/Target/X86/X86InstrAVX512.td
9277–9278

The name is not precise now. We now support non-fp14 node. Also update the comments.

9496

indent.

13488

Why not merge this class to avx512_fp14_p_vl_all? Is it because it doesn't use MXCSR?

13489

indent.

llvm/lib/Target/X86/X86InstrFoldTables.cpp
3037

Why no TB_NO_REVERSE for it?

craig.topper added inline comments.Aug 18 2021, 9:30 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
1929

rint and nearbyint are both C math library functions. rint raises an exception if the rounding isn't exact, nearbyint doesn't.

llvm/lib/Target/X86/X86InstrFoldTables.cpp
3037

Only the _Int need TB_NO_REVERSE because the memory type is 16 bits but the register class is VR128X so the sizes are different. The unfolding code would use the size of the register class to do the unfold and create a vmovaps/vmovups which would increase the size of the load.

For VSQRTZr, the register class is FR16X and the memory size is 16 bits so they match.

LuoYuanke added inline comments.Aug 18 2021, 7:18 PM
llvm/lib/Target/X86/X86ISelLowering.cpp
1929

Got it. Thanks. :)

llvm/lib/Target/X86/X86InstrFoldTables.cpp
3037

Got it. Thanks. :)

pengfei updated this revision to Diff 367426.Aug 19 2021, 1:56 AM

Address Yuanke's comments.

Thanks Yuanke and Craig.

llvm/lib/Target/X86/X86InstrAVX512.td
9277–9278

The precision of FP16 happens to 2-14 too, so the name is precise.

13488

Not only the use of MXCSR but also naming. Anyway, I merged them.

LuoYuanke accepted this revision.Aug 19 2021, 7:26 PM

LGTM, thanks. May wait 1 or 2 days for the comments from others.

This revision is now accepted and ready to land.Aug 19 2021, 7:26 PM
This revision was landed with ongoing or failed builds.Aug 21 2021, 5:59 PM
This revision was automatically updated to reflect the committed changes.