[RFC] Any-sized VT for TableGen

Hello,

I would like to gather some opinion about a feature we’d like to see added to TableGen: A Generic ValueType that only compares against other ValueTypes using their size.

Examples

  • vtAny32 represents any 32 bit type:
    • == to i32, f32, v2i16, v4i8, and so on.
  • vtAny64 represents any 64 bit type:
    • == to i64, f64, v2i32, and so on.

In DAG/GISel, they both result in a check on the other type’s size and not an equality check.
Note that we’d like to have more than just those two, ideally, we’d like a range of options between 16 and 1024.

Use cases

In the AMDGPU backend we have more than one case where the exact type of some node is irrelevant and we’d like to allow all N-sized types. Currently, we have to repeat the pattern or use a foreach, like so:

foreach vt = Reg32Types.types in {
   defm : DSReadPat_mc <DS_READ_B32, vt, "load_local">;
}

We would like to have something simpler, like

defm : DSReadPat_mc <DS_READ_B32, vtAny32, "load_local">;

Previous implementation attempt(s)

@arsenm has tried in the past to add them, but he got stuck with the DAG’s type inference. TableGen: Add size only type matchers · arsenm/llvm-project@eb1de38 · GitHub

I recently tried to rebase his work and give it another go, but I also got stuck during type inference. It’s (CodeGenDAGPatterns.cpp) a complex piece of code that’s not easy to understand - at least for me who has never worked on it. I tried not expanding the type (= not considering it as an overload) but it didn’t help much. I just end up with “Could not infer all types” errors no matter what changes I try.

Help Needed

  • Is this a feature you find interesting and would like to see, or not? Why?
  • Do you have any suggestion on how this should be implemented ?
    • Are we on the right track with adding new SimpleValueTypes & tweaking CodeGenDAGPatterns until it works? Or is there a more straightforward path we’re not seeing?
    • Should those types be overloaded types that are expanded? Or be their own thing?
      • If they should be overloaded, should we simply emit a OPC_SwitchType instead of adding a new “CheckTypeSize” opcode?

Thank you very much for your help!

1 Like

Hello, on the KVX backend (not upstreamed yet) we have lot of patterns like this:

defm : LoadPat<load, i64, 3, LDp, LDri10, LDri37, LDri64, LDrr>;
defm : LoadPat<load, v8i8, 3, LDp, LDri10, LDri37, LDri64, LDrr>;
defm : LoadPat<load, v2i32, 3, LDp, LDri10, LDri37, LDri64, LDrr>;
defm : LoadPat<load, v4i16, 3, LDp, LDri10, LDri37, LDri64, LDrr>;

Where LoadPat is a multiclass defining patterns for the various address-variants loads we have.
Indeed, on our architecture anything (of a given size) can fit in a register: floating-point value, integer value, vector, … But since TableGen patterns are strongly typed we have to duplicate those patterns.

Such feature would certainly reduce the number of patterns on our backend

I had worked on this during my past tenure at AMD. I and @arsenm had some conversation around it but we couldn’t take it ahead due to other priorities. Here is the PR.

It was created for sharing the code changes so please don’t mind hackiness in it and a lot of debug prints :slight_smile:
I am not sure if this is still useful but I thought of sharing it anyway.

Hi, did you manage to make it work reliably with type inference in the DAG Patterns?
That’s my biggest problem right now. Not sure if this should be an overloaded type or not, and if it’s not, I’m not sure how it should deal with inference.

I feel like it shouldn’t be overloaded as we want to emit special type-size checks for those types, but without overloading I don’t know where to add the special cases to support inferring such a type