[Mlir-commits] [mlir] [MLIR][Conversion] XeGPU to XeVM: create_nd_tdesc - Add support for base memory rank > 2 (PR #164701)

[Charitha Saumya]

charithaintc wrote:

> > > result type of create_nd_tdesc is lowered to single i64 value.
> > 
> > 
> > Any specific reason for this? why not increase the payload size to accommodate additional ranks information. I think having a payload is useful to retrieve info at the consumer. I would reconsider this unless we plan to get rid of createNd and move everything to consumer side.
> 
> Several reasons:
> 
> * Payload cannot be just 2D fixed type. Offset calculation needs to be done as part of lowering consumer op and it needs source rank information and shape/strides. Amount of info is variable depending on source.
> * Type convertor cannot get source rank info when deciding lowered type. Lowered payload type has to be generic, independent of rank. Structure will be similar to that of unranked memref.
> * Producer - create_nd_tdesc - and consumers have direct def-use connection. All information regarding source memory for offset computation used for consumer op can be directly accessed through create_nd_tdesc op in the consumer op lowering pattern. Creating a payload just replicates this information and makes it available for access at runtime at XeVM level. It does not provide any additional benefit.
> * Using payload can makes compile time folding and other optimizations difficult to do. Test cases in this PR calls canonicalize and cse to show the benefit of not having a payload structure. If shape and strides are compile-time constants, entire compute chain for offset computation gets optimized to constants as shown in   https://github.com/llvm/llvm-project/pull/164701/files#diff-c29451f2af1cb1d5540fe09d3b8056829598407289249e2165908cb3b075883c  and even if shape and strides are not constant, if multiple tensor descriptors have same shape and strides, common compute can be shared as shown in  https://github.com/llvm/llvm-project/pull/164701/files#diff-e16a5efc3f4f94de5091c890907f30ad57dbd3af2be49b0ce9d5e954acb79e13

I see. 

In that case, can you clarify how it is handled when tensor_desc is a block arg or a func arg? Sorry I did not have time to a closer look at the changes yet. 

I feel like after this change, create_nd does not really give us any value, rather it becomes a burden because we have to look it up during lowering. We can simply move all info to loadNd to simplify things. We should consider removing create_nd in that case.  

https://github.com/llvm/llvm-project/pull/164701