spall 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?

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

Reply via email to