[PATCH] D139301: [X86] Add scheduling info of CodeGenOnly but encodable instructions for AlderlakeP model

[Haohai, Wen via Phabricator via llvm-commits]

HaohaiWen marked 2 inline comments as done.
HaohaiWen added inline comments.


================
Comment at: llvm/lib/Target/X86/X86SchedAlderlakeP.td:812-814
+def : InstRW<[ADLPWriteResGroup38, ReadInt2Fpu], (instrs CVTSI642SSrr)>;
+def : InstRW<[ADLPWriteResGroup38, ReadDefault, ReadInt2Fpu], (instregex "^(V?)CVTSI642SSrr_Int$")>;
+def : InstRW<[ADLPWriteResGroup38, ReadDefault, ReadInt2Fpu], (instrs VCVTSI642SSrr)>;
----------------
andreadb wrote:
> HaohaiWen wrote:
> > RKSimon wrote:
> > > HaohaiWen wrote:
> > > > CVTSI642SSrr has different schedreadwrite with CVTSI642SSrr_Int?
> > > > I think they should be same.
> > > Difference between sse12_cvt_s and sse12_vcvt_avx that are getting interpreted differently for some reason?
> > Hi @andreadb, could you please answer RKSimon's question? I noticed you edited it in D57148.
> It has been a while since I looked at that .td file. However, I think that the main difference is that some variants accept two operands, while other variants use three-operands forms.
> 
> More specifically, _Int variants tend to use three operands. CVTSI642SS is a codegen only instance of multiclass sse12_cvt_s, which assumes two input operands:
> 
> ```
>   def rr : SI<opc, MRMSrcReg, (outs DstRC:$dst), (ins SrcRC:$src),
>               !strconcat(asm,"\t{$src, $dst|$dst, $src}"),
>               [(set DstRC:$dst, (OpNode SrcRC:$src))]>,
>               Sched<[sched, Int2Fpu]>;
> ```
> 
> That's why we don't need to skip the first use by introducing an "extra" ReadDefault.
> 
> If you look at the tablegen'd definitions, you see this:
> ```
>   { 909,        2,      1,      0,      613,    0|(1ULL<<MCID::MayRaiseFPException), 0x1508023029ULL, ImplicitList18, nullptr, OperandInfo205 },  // Inst #909 = CVTSI642SSrr
>   { 910,        3,      1,      0,      614,    0|(1ULL<<MCID::MayRaiseFPException), 0x1508023029ULL, ImplicitList18, nullptr, OperandInfo204 },  // Inst #910 = CVTSI642SSrr_Int
> ```
> 
> Note how CVTSI642SSrr takes 2 operands (second field), but the _Int variant takes 3 operands.
```
multiclass sse12_cvt_sint_3addr<bits<8> opc, RegisterClass SrcRC,
                    RegisterClass DstRC, X86MemOperand x86memop,
                    string asm, string mem, X86FoldableSchedWrite sched,
                    Domain d, bit Is2Addr = 1> {
let hasSideEffects = 0, ExeDomain = d in {
  def rr_Int : SI<opc, MRMSrcReg, (outs DstRC:$dst), (ins DstRC:$src1, SrcRC:$src2),
                  !if(Is2Addr,
                      !strconcat(asm, "\t{$src2, $dst|$dst, $src2}"),
                      !strconcat(asm, "\t{$src2, $src1, $dst|$dst, $src1, $src2}")),
                  []>, Sched<[sched, ReadDefault, ReadInt2Fpu]>;
  let mayLoad = 1 in
  def rm_Int : SI<opc, MRMSrcMem, (outs DstRC:$dst),
                  (ins DstRC:$src1, x86memop:$src2),
                  !if(Is2Addr,
                      asm#"{"#mem#"}\t{$src2, $dst|$dst, $src2}",
                      asm#"{"#mem#"}\t{$src2, $src1, $dst|$dst, $src1, $src2}"),
                  []>, Sched<[sched.Folded, sched.ReadAfterFold]>;
}
}

```
Should we use Sched<[sched, ReadInt2Fpu] if Is2Addr  == 1?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139301/new/

https://reviews.llvm.org/D139301