jasilvanus wrote: @shiltian @arsenm Sorry for the late feedback, I missed this. I think this deprecation was considered from a compute perspective, and there are graphics use cases in dynamic VGPR mode that still need an explicit VGPR limit. (I wonder whether in the future there also could be compute use cases if compute used dVGPR.)
My concern is the backend support for the IR attribute `"amdgpu-num-vgpr"` only, which `AMDGPUUsage.rst` now marks as deprecated. I'm not concerned about the clang changes. I think we need to keep supporting the VGPR limit attribute. Cases where a VGPR limit is needed for correctness: 1. Fallback functions passed to dVGPR chain calls. These are invoked with no guarantee about the number of VGPRs currently held, so they must work when holding only a single VGPR block. The only safe way for a frontend to emit them is with a limit of 16 VGPRs. 2. Some use cases may need a limit to guarantee forward progress. And for performance: The frontend may need to limit certain functions to a low limit (e.g. one VGPR block). `amdgpu_waves_per_eu` is not an adequate replacement here. In dVGPR mode occupancy is a dynamic property, so waves-per-SIMD isn't a meaningful way to express this. And for low limits there is no corresponding static occupancy at all: On GFX12 a 16-VGPR limit would correspond to 96 waves per SIMD, far beyond the available wave slots. (Related: #226242 added a single-block limit for dVGPR entry functions by clamping internally in `GCNSubtarget::getMaxNumVGPRs` rather than via the attribute. That one doesn't strictly need the attribute, but it's the same underlying need.) https://github.com/llvm/llvm-project/pull/193023 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
