bob80905 wrote: > > > this is not NFC, so we should verify that we can call these intrinsics > > > with `half` values even if 16-bit types aren't enabled, and that they > > > properly codegen to 32-bit varia > > > > > > > > > > > For example, for `abs`, it still depends on the > > > > > > > _HLSL_16BIT_AVAILABILITY availability attribute. Does this PR > > > > > > > intend to keep abs overloads using half "unexposed"? Or should > > > > > > > that overload for abs be exposed too? > > > > > > > > > > > > > > > > > > Oh I see what you are referring to. That might be my mistake; let > > > > > > me double check if I used the wrong __HLSL_AVAILABILITY > > > > > > > > > > > > > > > Yeah, if the intention of this PR is to expose all half type > > > > > overloads always, then I would think there is no more utility in > > > > > defining `HLSL_16BIT_AVAILABILITY` anymore. > > > > > > > > > > > > I think I was meant to use _HLSL_16BIT_AVAILABILITY and not > > > > _HLSL_AVAILABILITY; I'll fix that. Thanks for noticing the error! > > > > > > > > > What I'm trying to get at is, you're removing #ifdef > > > __HLSL_ENABLE_16_BIT, and replacing it with an availability attribute > > > making sure that the shader model is at least 6.2 for these functions. If > > > you look at the definition for HLSL_16bit_availability, you'll notice > > > that it's also defined under an ifdef: #ifdef __HLSL_ENABLE_16_BIT The > > > essence of this PR seems to me like it's assuming from now on that #ifdef > > > __HLSL_ENABLE_16_BIT will always be true. So, that means this > > > availability attribute will always be defined. And it's identical to the > > > HLSL_AVAILABILITY attribute. I might understand this incorrectly, but now > > > it seems to me like there's no point or distinguishing use between > > > HLSL_AVAILABILITY and HLSL_16bit_AVAILABILITY. Does that make sense? > > > Which would imply that functions like `abs` should just use > > > HLSL_Availability instead and we can do away with > > > HLSL_16bit_AVAILABILITY. Since you want all function overloads with a > > > half parameter to always be exposed. > > > > > > @llvm-beanz Thoughts on this? > > Looking at this again' _HLSL_16BIT_AVAILABILITY' is defined even if > _HLSL_ENABLE_16_BIT isn't defined; the important thing is the overloads are > defined even if there is no native half.
Nevermind I missed the #else :P All resolved. https://github.com/llvm/llvm-project/pull/132804 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
