[PATCH] D46356: [TableGen] Emit a fatal error on inconsistencies in resource units vs cycles.

[Clement Courbet via Phabricator via llvm-commits]

courbet added a comment.

In https://reviews.llvm.org/D46356#1085339, @javed.absar wrote:

> In https://reviews.llvm.org/D46356#1085333, @courbet wrote:
>
> > In https://reviews.llvm.org/D46356#1085328, @RKSimon wrote:
> >
> > > I think it'd be clearer if we could keep Btver2/Znver1 to the Res = [1] default tbh
> >
> >
> > Hm, not sure how I can do that without triggering the check for  cases when the user erroneously specified `[1]` when they wanted to use the default (now `[]`). Did I miss a possibility ?
> >  This makes is no difference for the user, but allows more checking. Best of both worlds ? :)
>
>
> I am bit confused on - "user erroneously specified `[1]` when they wanted to use the default ". But the default was always [1]


Now that we're checking what the user wrote, we want the following cases to be valid:

  def : WriteRes<SchedRW, [P0, P1]> {
      let ResourceCycles = [1, 1];  // explicit
  }
  
  def : WriteRes<SchedRW, [P0, P1]> {
      let ResourceCycles = [];  // explicitly use default, same as let ResourceCycles = [1,1]
  }
  
  def : WriteRes<SchedRW, [P0, P1]> {
      // implicitly use default, same as let ResourceCycles = [1,1]
  }

But this one to be invalid:

  def : WriteRes<SchedRW, [P0, P1]> {
      let ResourceCycles = [1];  // Did the user expect [1,1]; or did they make a mistake ? Throw an error.
  }

In this specific case we want:

  defm : ZnWriteResPair<WriteIMul,   [ZnALU1, ZnMultiplier], 4>;            // (A) ok, [1,1]
  defm : ZnWriteResPair<WriteIMul,   [ZnALU1, ZnMultiplier], 4, [1,1]>;  // (B) ok, [1,1]
  defm : ZnWriteResPair<WriteIMul,   [ZnALU1, ZnMultiplier], 4, [1]>;     // (C) Error, not sure if this is a mistake or not.

If I keep the default `Res = [1]` in the definition of ZnWriteResPair, I cannot make a difference between (A) and (C). Using the the default `Res = []` allows making the difference and is consistent with what WriteRes uses as a default.

Am I making more sense ?


Repository:
  rL LLVM

https://reviews.llvm.org/D46356