This is an archive of the discontinued LLVM Phabricator instance.

[x86] add vmovss/sd missing encoding
ClosedPublic

Authored by AsafBadouh on Nov 16 2015, 4:38 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

AsafBadouh updated this revision to Diff 40275.Nov 16 2015, 4:38 AM
AsafBadouh retitled this revision from to [x86] add vmovss/sd missing encoding .
AsafBadouh updated this object.
AsafBadouh added reviewers: delena, igorb.
AsafBadouh added a subscriber: llvm-commits.
delena edited edge metadata.Nov 17 2015, 3:43 AM

Let's add intrinsics and create patterns with "maskable".

AsafBadouh updated this revision to Diff 41392.Nov 30 2015, 5:06 AM
AsafBadouh edited edge metadata.

add intrinsics

igorb edited edge metadata.Dec 1 2015, 6:34 AM

please add encoding tests for intel style assembler.
Is it possible to implement movss/sd load intrinsic in this patch?

../trunk/lib/Target/X86/X86InstrAVX512.td
2957 ↗(On Diff #41392)
  1. dest is also source in case k[0] == 0
  2. (ins _.MemOp:$src) should be (ins _.ScalarMemOp:$src)
  3. please add mayLoad = 1

something like

let Constraints = "$src1 = $dst" , mayLoad = 1 in
  defm rm_Int : AVX512_maskable_3src_scalar<0x10, MRMSrcMem, _, 
                  (outs _.RC:$dst), 
                  (ins _.ScalarMemOp:$src),
                  asm,"$src","$src",
                  (_.VT (OpNode (_.VT _.RC:$src1), 
                                (_.VT (scalar_to_vector
                                        (_.ScalarLdFrag addr:$src)))))>, EVEX;
2963 ↗(On Diff #41392)

You can use AVX512PI , and pass vmovss as parameter to avx512_move_scalar to avoid "v"#asm above ( in rr_Int )

2968 ↗(On Diff #41392)

let mayLoad = 1 in

AsafBadouh updated this revision to Diff 41645.Dec 2 2015, 10:27 AM
AsafBadouh edited edge metadata.

made changes according to Igor comments
plan to add load intrinsics in the following patch
add intel style encoding tests
thanks for the review!

delena accepted this revision.Dec 3 2015, 3:24 AM
delena edited edge metadata.

LGTM

../trunk/lib/Target/X86/X86IntrinsicsInfo.h
810 ↗(On Diff #41392)

the line is too long

This revision is now accepted and ready to land.Dec 3 2015, 3:24 AM
This revision was automatically updated to reflect the committed changes.