[PATCH] D46356: [TableGen] Emit a fatal error on inconsistencies in resource units vs cycles.
Evandro Menezes via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 4 08:45:00 PDT 2018
evandro added a comment.
In https://reviews.llvm.org/D46356#1087529, @courbet wrote:
> @evandro Do you have benchmarks to evaluate the impact of this change ?
Yes. They're not done yet. Will let you know when I get the results.
Thank you.
================
Comment at: lib/Target/AArch64/AArch64SchedExynosM3.td:307
let NumMicroOps = 1;
- let ResourceCycles = [8]; }
+ let ResourceCycles = [8, 1]; }
def M3WriteNEONW : SchedWriteRes<[M3UnitFDIV,
----------------
courbet wrote:
> evandro wrote:
> > courbet wrote:
> > > evandro wrote:
> > > > courbet wrote:
> > > > > evandro wrote:
> > > > > > Please, make this [8, 8]...
> > > > > Note that the two following definitions are strictly equivalent:
> > > > >
> > > > > ```
> > > > > def M3WriteNEONV : SchedWriteRes<[M3UnitFDIV, M3UnitFDIV]> {
> > > > > let Latency = 7;
> > > > > let NumMicroOps = 1;
> > > > > let ResourceCycles = [8,8];
> > > > > }
> > > > > ```
> > > > >
> > > > > ```
> > > > > def M3WriteNEONV : SchedWriteRes<[M3UnitFDIV]> {
> > > > > let Latency = 7;
> > > > > let NumMicroOps = 1;
> > > > > let ResourceCycles = [16];
> > > > > }
> > > > > ```
> > > > >
> > > > > What's the semantics that you're trying to express by splitting into `8*M3UnitFDIV + 8*M3UnitFDIV` vs `16*M3UnitFDIV` ? The SubtargetEmitter is destroying these semantics anyway.
> > > > >
> > > > > For reference before this change the definition is equivalent to:
> > > > >
> > > > > ```
> > > > > def M3WriteNEONV : SchedWriteRes<[M3UnitFDIV]> {
> > > > > let Latency = 7;
> > > > > let NumMicroOps = 1;
> > > > > let ResourceCycles = [9];
> > > > > }
> > > > > ```
> > > > >
> > > > >
> > > > >
> > > > There are two FDIV units in M3:
> > > >
> > > > def M3UnitFDIV : ProcResGroup<[M3UnitFDIV0, M3UnitFDIV1]>;
> > > >
> > > Yes indeed, but this is already covered by the M3UnitFDIV referencing these two.
> > >
> > > I'm not familiar with Exynos, what's the semantics that you're trying to express here ?
> > >
> > > As written before this change, M3WriteNEONV dispatches one Uop that writes its output with latency 7 and uses either of {M3UnitFDIV0, M3UnitFDIV1} for 9 cycles.
> > > After that change, whatever way (`[8,8]` or `[16]`) we write it, M3WriteNEONV dispatches one Uop that writes its output with latency 7 and uses either of {M3UnitFDIV0, M3UnitFDIV1} for 16 cycles.
> > >
> > > Maybe that's the case, but since that sounds weird, I just want to confirm.
> > >
> > >
> > >
> > >
> > I mean that with `M3WriteNEONV`, for `FDIVv4f32`, one uop is dispatched to both `[M3UnitFDIV0, M3UnitFDIV1]` (don't ask me how this is done inside the machine) and each used up for 8 cycles.
> Thanks for the explanation. So you wanted to write:
>
> ```
> def M3WriteNEONV : SchedWriteRes<[M3UnitFDIV0, M3UnitFDIV1]> {
> let Latency = 7;
> let NumMicroOps = 2;
> let ResourceCycles = [8,8];
> }
> ```
>
> I've updated the diff with that.
>
>
>
>
>
Thank you.
================
Comment at: lib/Target/AArch64/AArch64SchedExynosM3.td:311
let NumMicroOps = 1;
- let ResourceCycles = [13]; }
+ let ResourceCycles = [13, 1]; }
def M3WriteNEONX : SchedWriteRes<[M3UnitFSQR,
----------------
evandro wrote:
> evandro wrote:
> > [13, 13]...
> And here I mean that with `M3WriteNEONV`, for `FDIVv2f64`, one uop is dispatched to both `[M3UnitFDIV0, M3UnitFDIV1]` and each used up for 13 cycles.
Err, I meant `M3WriteNEONW` here. Thank you.
================
Comment at: lib/Target/AArch64/AArch64SchedExynosM3.td:376
let NumMicroOps = 2;
- let ResourceCycles = [5]; }
+ let ResourceCycles = [5, 1]; }
def M3WriteVLDG : SchedWriteRes<[M3UnitL,
----------------
courbet wrote:
> This still feels weirdly asymetric. Is that correct ?
No, it should be `[5, 5]`, thank you.
Repository:
rL LLVM
https://reviews.llvm.org/D46356
More information about the llvm-commits
mailing list