[RISCV][SiFive7Model] Best way to provide custom VCIX latencies?

[michalt]

I'm not sure if this belongs to a bug, but I wanted to open the conversation about the best approach to dealing with scheduling of VCIX instructions. What I'd like to be able to do is to:

• Be able to provide accurate latencies of VCIX instructions in our internal LLVM build. (NB: These latencies would not be upstreamed)
• But reuse the upstream SiFive7Model.
• Make it reasonably easy to keep up with the upstream changes of SiFive7Model (e.g., manually copy & paste of the VCIX latencies would be ok, but it'd be nice to avoid anything more complicated).

What would be the best approach to achieve this?

Idea 1:

• Create a copy of RISCVSchedSiFive7.td (let's call it RISCVSchedSiFive7Foobar.td).
• But this leads to name conflicts since everything is just included in RISCV.td.
• So one could run sed s/SiFive7/SiFive7Foobar/g over the file. This is not quite enough (VLDSX0Pred conflicts); perhaps we could try to use the SiFive7 prefix for all definitions in that file? Would that be acceptable?
• The above should be mostly mechanical (could be automated by a script). Then it would be a matter of (semi-)manually adjusting the VCIX scheduling information.

Idea 2:

• Perhaps we could refactor RISCVSchedSiFive7.td to have something like SiFive7ModelTemplate that would be a multiclass and could be instantiated with different VCIX scheduling information? (i.e., we would just instantiate it to create the current  SiFive7Model)
• This seems like a cleaner approach where RISCVSchedSiFive7Foobar.td would not have to be a copy (with all the sed renaming), it would just instantiate the model and provide the custom VCIX latencies.
• But I'm not sure if this is even feasible? (not a TableGen expert!) Would we run into some issues with this? Would it significantly increase the maintenance burden of RISCVSchedSiFive7.td?

@topperc @michaelmaitland What are your thoughts? Any better ideas would be appreciated! 🙂 

[llvmbot]

@llvm/issue-subscribers-backend-risc-v

Author: Michal Terepeta (michalt)

I'm not sure if this belongs to a bug, but I wanted to open the conversation about the best approach to dealing with scheduling of VCIX instructions. What I'd like to be able to do is to:

• Be able to provide accurate latencies of VCIX instructions in our internal LLVM build. (NB: These latencies would not be upstreamed)
• But reuse the upstream SiFive7Model.
• Make it reasonably easy to keep up with the upstream changes of SiFive7Model (e.g., manually copy & paste of the VCIX latencies would be ok, but it'd be nice to avoid anything more complicated).

What would be the best approach to achieve this?

Idea 1:

• Create a copy of RISCVSchedSiFive7.td (let's call it RISCVSchedSiFive7Foobar.td).
• But this leads to name conflicts since everything is just included in RISCV.td.
• So one could run sed s/SiFive7/SiFive7Foobar/g over the file. This is not quite enough (VLDSX0Pred conflicts); perhaps we could try to use the SiFive7 prefix for all definitions in that file? Would that be acceptable?
• The above should be mostly mechanical (could be automated by a script). Then it would be a matter of (semi-)manually adjusting the VCIX scheduling information.

Idea 2:

• Perhaps we could refactor RISCVSchedSiFive7.td to have something like SiFive7ModelTemplate that would be a multiclass and could be instantiated with different VCIX scheduling information? (i.e., we would just instantiate it to create the current  SiFive7Model)
• This seems like a cleaner approach where RISCVSchedSiFive7Foobar.td would not have to be a copy (with all the sed renaming), it would just instantiate the model and provide the custom VCIX latencies.
• But I'm not sure if this is even feasible? (not a TableGen expert!) Would we run into some issues with this? Would it significantly increase the maintenance burden of RISCVSchedSiFive7.td?

@topperc @michaelmaitland What are your thoughts? Any better ideas would be appreciated! 🙂 

[wangpc-pp]

I was always thinking we need function/lambda to be supported in TableGen to increase maintainability of RVV scheduling. I have a PoC (https://reviews.llvm.org/D148915) before I haven't got any time to push it forward.

[michaelmaitland]

We have faced the problem of configurability of scheduler models in our downstream. It is very similiar to Idea 2 that @michalt suggests. Here's what we did to address it:

1. Define a SiFive7SchedMachineModel which accepts configuration parameters:

class SiFive7SchedMachineModel<bit hasXXX, bit hasYYY, ...> : SchedMachineModel {
  ...  // all the let statements
  bit HasXXX = hasXXX;
  bit HasYYY = hasYYY;
  ... // rest of the configuration related flags
}

2. Instantiate models based on configurations we are interested in supporting:

def SiFive7ModelA : SiFive7SchedMachineModel<true, true, ...>;
def SiFive7ModelB: SiFive7SchedMachineModel<true, false, ...>;

3. Create parameterized SiFive7SchedResources class that depends on configuration parameters:

class SiFive7SchedResources<bit hasXXX, bit hasYYY> {
  // define proc resources, writeres, and readadvances. Use configuration params as necessary.
}

for model = [SiFive7ModelA, SiFive7ModelB] in {
  defvar name = !if(model.HasXXX, "XXX", "") # !if(model.HasYYY, "YYY", "");
  let SchedModel = model in
    defm name : SiFiveSchedResources<model.HasXXX, model.HasYYY>;
}

In either case, I think we could probably make progress with the above approach without lambdas and if the lambda stuff gets worked on in the future and it makes sense to use them here, we can always refactor.

[michalt]

I just noticed that a recent refactoring (7a33569) is trying to make the model definition easier to parameterize, e.g., "FP latencies" are provided as an argument to SiFive7SchedResources and SiFive7WriteResBase.

@mshockwave @michaelmaitland Do you think we could do something like that for the VCIX instructions?

[mshockwave]

I just noticed that a recent refactoring (7a33569) is trying to make the model definition easier to parameterize, e.g., "FP latencies" are provided as an argument to SiFive7SchedResources and SiFive7WriteResBase.

@mshockwave @michaelmaitland Do you think we could do something like that for the VCIX instructions?

7a33569 indeed follows (and credits to) @michaelmaitland 's design in the previous comment. I believe it will be easier for users to create their custom scheduling models and configure accordingly. However, I this won't allow use to assign different scheduling data (i.e. latencies and occupancies) based on the instruction payloads -- which will be something nice to have for VCIX -- so the entire class of VCIX instructions (e.g. WriteVC_IV_M1) will share the same scheduling data. This is the limitation of LLVM's scheduling model and solving it will take much more efforts.

That being said, I'm more than happy to factor out VCIX instructions' data into a separate entity (class) similar to FP latencies, so that we can at least have some flexibilities, albeit not a perfect solution.