erichkeane wrote: > Thank you Erich for the comments showing type of array. That helped! The > changes look great to me! >
Awesome, thanks! I'll still be doing a patch to do the recipe-ordering cleanup, but that should be effectively NFC, but I'll do that after this gets merged. > A few semi-relevant comments: > > * I noticed you use the upperbound instead of extent in the recipes - > despite the fact that only extent is filled in in the bounds used in the > `acc.private` operations. I am guessing you will be relying on the promise I > made (but not yet implemented) - > https://mlir.llvm.org/docs/Dialects/OpenACCDialect/#accget_upperbound-accgetupperboundop > Yep :) Thats why i was so excited about it! :) the loops worked out way better doing LB->UB > * It is expected that you are not using stride from `acc.bounds` (since > OpenACC spec does not actually allow stride). We currently do use this in > flang because the language can express non-contiguous array views. I am > imagining that if ever mdspan was natively represented in compiler as some > sort of built-in type, then using stride would be necessary. Right, the standard doesn't allow stride, and C/C++ doesn't really have a non-1 stride available in its syntax. Yes, a 'native' mdspan would require some sort of stride operation, but I don't see us able ot do that anytime soon. > > * I noticed you collapse multi-dimension arrays into a single loop. I > imagine normal declarations do not result in non-contiguous arrays. However, > does your semantics checking verify for contiguous sections in the case when > acc bounds are used? Multi-dimension arrays in C/C++ are guaranteed contiguous. Where that doesn't happen is when there are pointers involved, but we have to 'set that up' with allocas, which I'll do in a subsequent patch. In those cases, we end up having to emit the values better, but you'll see that in upcoming patches. The ptr_stride operations here are 'pointer aware' so they should do the indirections as necessary. https://github.com/llvm/llvm-project/pull/160189 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
