This is an archive of the discontinued LLVM Phabricator instance.

[X86] Correct the scheduling information for AVX-VNNI and AVX512-VNNI instructons.
ClosedPublic

Authored by craig.topper on Apr 8 2023, 7:53 PM.

Details

Summary

The AVXVNNI load instructions weren't using the Folded load write
class and they had no ReadAdvance.
The YMM versions were using the XMM schedule class.

The AVX512VNNI instructions had the right classes, but not enough
ReadAdvances to account for the 2 sources.

Noticed while investigating #62026.

Diff Detail

Event Timeline

craig.topper created this revision.Apr 8 2023, 7:53 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 8 2023, 7:53 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
craig.topper requested review of this revision.Apr 8 2023, 7:53 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 8 2023, 7:53 PM
craig.topper edited the summary of this revision. (Show Details)Apr 8 2023, 7:56 PM

had the write classes

Do you mean right?

llvm/lib/Target/X86/X86InstrAVX512.td
12580

It's a mystery to me that AVX512 has a lot of 3 ops instructions but not even have one ReadAfterFold, in X86InstrAVX512.td

grep -rn 'ReadAfterFold,' llvm/lib/Target/X86/X86InstrAVX512.td
craig.topper edited the summary of this revision. (Show Details)Apr 8 2023, 11:41 PM

had the write classes

Do you mean right?

Yes. Fixed. Thanks!

craig.topper added inline comments.Apr 8 2023, 11:45 PM
llvm/lib/Target/X86/X86InstrAVX512.td
12580

Yeah just from a quick check, vpermt2 vpermi2, and vpternlog are also wrong.

Also interesting, EVEX FMA instructions are wrong but are correct for the VEX versions in X86InstrFMA.td

pengfei accepted this revision.Apr 9 2023, 1:29 AM

LGTM.

This revision is now accepted and ready to land.Apr 9 2023, 1:29 AM
LuoYuanke added inline comments.Apr 9 2023, 3:54 PM
llvm/lib/Target/X86/X86InstrAVX512.td
12589

@craig.topper, I don't understand why it needs another sched.ReadAfterFold? Is it because there are 3 operands?

craig.topper added inline comments.Apr 9 2023, 4:03 PM
llvm/lib/Target/X86/X86InstrAVX512.td
12589

Yes each read is associated with an operand. So we need one for each vector source.

LuoYuanke added inline comments.Apr 9 2023, 4:50 PM
llvm/lib/Target/X86/X86InstrAVX512.td
12589

Why in line 12580, for rr version, there is only one sched (WriteVecIMulX) in the sched list?

LuoYuanke added inline comments.Apr 9 2023, 4:52 PM
llvm/lib/Target/X86/X86InstrAVX512.td
12589

I mean

defm r  :   AVX512_maskable_3src<Op, MRMSrcReg, VTI, (outs VTI.RC:$dst),
                                 (ins VTI.RC:$src2, VTI.RC:$src3), OpStr,
                                 "$src3, $src2", "$src2, $src3",
                                 (VTI.VT (OpNode VTI.RC:$src1,
                                          VTI.RC:$src2, VTI.RC:$src3)),
                                 IsCommutable, IsCommutable>,
                                 EVEX_4V, T8PD, Sched<[sched]>;
pengfei added inline comments.Apr 10 2023, 8:22 AM
llvm/lib/Target/X86/X86InstrAVX512.td
12589