This is an archive of the discontinued LLVM Phabricator instance.

[X86][AVX512] fix dag & add intrinsics for fixupimm
ClosedPublic

Authored by AsafBadouh on Jan 19 2016, 4:25 AM.

Details

Summary

cover all width and types (pd/ps/sd/ss) of fixupimm instruction and inrtinsics

Diff Detail

Repository
rL LLVM

Event Timeline

AsafBadouh updated this revision to Diff 45242.Jan 19 2016, 4:25 AM
AsafBadouh retitled this revision from to [X86][AVX512] fix dag & add intrinsics for fixupimm.
AsafBadouh updated this object.
AsafBadouh added reviewers: m_zuckerman, igorb, delena.
AsafBadouh set the repository for this revision to rL LLVM.
AsafBadouh added a subscriber: Restricted Project.
AsafBadouh edited subscribers, added: llvm-commits; removed: Restricted Project.
igorb added inline comments.Jan 19 2016, 4:44 AM
E:/Work/LLVM/trunk/lib/Target/X86/X86InstrAVX512.td
7529 ↗(On Diff #45242)

why _.MemOp and not _.ScalarMemOp ? If you change it i think you can remove defm rmi_alt

E:/Work/LLVM/trunk/test/CodeGen/X86/avx512-intrinsics.ll
7046 ↗(On Diff #45242)

please remove empty lines.

E:/Work/LLVM/trunk/test/CodeGen/X86/avx512vl-intrinsics.ll
7708 ↗(On Diff #45242)

could you please remove zeroinitializer from the tests? It only complicate checks.

7753 ↗(On Diff #45242)

please remove empty lines

AsafBadouh marked 3 inline comments as done.Jan 19 2016, 5:06 AM
AsafBadouh added inline comments.
E:/Work/LLVM/trunk/test/CodeGen/X86/avx512vl-intrinsics.ll
7708 ↗(On Diff #45242)

I think that this zeroinitializer is necessary since you can create fixupimmup {k}{z} with the non-maskz intrinsic version, and that case check it's ok.
unlike the zeroinitializer in line 7726, there, the zeroinitializer is just another argument.

what do you think?

igorb added inline comments.Jan 19 2016, 5:50 AM
E:/Work/LLVM/trunk/test/CodeGen/X86/avx512vl-intrinsics.ll
7708 ↗(On Diff #45242)

ok

AsafBadouh updated this revision to Diff 45248.Jan 19 2016, 6:00 AM

fixes according to Igor comments.

igorb accepted this revision.Jan 19 2016, 6:03 AM
igorb edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jan 19 2016, 6:03 AM
This revision was automatically updated to reflect the committed changes.