[PATCH] D150310: [TableGen][SubtargetEmitter] Add the StartAtCycles field in the WriteRes class.

[Francesco Petrogalli via Phabricator via llvm-commits]

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


================
Comment at: llvm/utils/TableGen/SubtargetEmitter.cpp:1198
+          if (StartAtCycles[PRIdx] > Cycles[PRIdx]) {
+            PrintFatalError(WriteRes->getLoc(),
+                            Twine("Inconsistent resource cycles: StartAtCycles "
----------------
michaelmaitland wrote:
> michaelmaitland wrote:
> > Do we need this check?
> > 
> > I am trying to model the following case:
> > 
> > I have two resources, `ResA` and `ResB`. I'd like it to be the case that `ResA` models the first of a two stage resource where each stage takes one cycle. When the first stage is not being used, another instruction can enter into the first stage. We can model this by letting `ResA` represent the first stage, and setting `ResourceCycles=1` for `ResA` and `Latency=2+LatencyFromResB` (The 2 represents 1 cycle from the first stage and 1 cycle from the second stage).
> > 
> > I'd like to model that `ResB` is used for one cycle, only after both stages of the resource have finished. In other words, this is two cycles from the start of issuing the instruction. Using `StartAtCycles` I write something like this:
> > ```
> > let ResourceCycles = [1, 1], StartAtCycles =[0, 2] in
> >   WriteRes<ResourceName, [ResA, ResB]>;
> > ```
> > 
> > Here, I run into this FatalError. Am I misunderstanding the semantics of StartAtCycle? Is there a better way to accomplish what I'm after?
> When I read the comment I now understand:
> 
> ```
> /// TODO: consider renaming the field `StartAtCycle` and `Cycles` to
> /// `AcquireAtCycle` and `ReleaseAtCycle` respectively, to stress the
> /// fact that resource allocation is now represented as an interval,
> /// relatively to the issue cycle of the instruction.
> ```
> 
> Maybe it would be a good idea to make this change as it is much easier to understand whats going on when we use this naming. I will prepare a patch.
@michaelmaitland  - thank you for addressing the TODO in https://reviews.llvm.org/D158568


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150310