https://github.com/easyonaadit commented:
For add/sub + bitwise operations, it will be easier for users of this builtin to have one overloaded function rather than type suffixed ones. The min/max case is a bit more challenging simply because min/max behavior is more challenging: https://llvm.org/docs/LangRef.html#floating-point-min-max-intrinsics-comparison We want to let the user know that floating min/max has some quirks, and let them choose their own expected behaviour. If builtins for the other types aren't going to have the type suffix, then for the sake of uniformity even the min/max shouldn't make use of it. ie, `__builtin_amdgcn_wave_reduce_min(input_val, stratergy, [fmin/fminimum/fminum])` seems like a better option than having `__builtin_amdgcn_wave_reduce_[fmin/fminum/fminimum](input_val, stratergy)`. > Given the point above I suspect you envision reworking the existing > intrinsics themselves, but that'd be a separate piece of work. The behavior of the intrinsics these builtins lower to, have been documented in the usage.rst file. ATM these are modelled similar to `fminnum`, but if and when other implementations are added, its just a matter of updating the documentation for the builtin. As long as the builtins themselves clearly document the intended output they will produce, this seems like a welcome change. > I'm not sure I follow what generic in the typical target sense is meant to > mean, but generic itself is an overloaded term and we might be relying on > different sources for our nomenclature. Whilst I would object to naming this > "the great munging", I'm sure I can reword it in a less open to > interpretation way, thank you for the suggestion. Taking a cursory glance at the title, "_generic_" implies "_something common to all targets in llvm_", which is not true in this case. Also "_ops_" isn't the right word for this change, there are actual `wave_reduce_pseudo` opcodes in the backend. Maybe you could try something along the lines of: "Remove type suffix from `wave_reduce` builtins" https://github.com/llvm/llvm-project/pull/179589 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
