================
@@ -9102,6 +9102,15 @@ bool InitializationSequence::Diagnose(Sema &S,
 
   case FK_ConversionFailed: {
     QualType FromType = OnlyArg->getType();
+    // __amdgpu_feature_predicate_t can be explicitly cast to the logical op
+    // type, although this is almost always an error and we advise against it
----------------
AaronBallman wrote:

> The feature builtins allow a linear, inline transform from ... to ... which 
> is easy to teach / adopt and matches what folks are already doing, and just 
> works as is, both on concrete, compile-time finalised targets, and on 
> abstract run-time finalised ones. 

They work as-is so long as you hold them right. What I think is not easy to 
teach or adopt is the fact that it's tied so tightly to optimizer behavior in 
surprising ways. The fact that these two are not necessarily equivalent (esp as 
the logic becomes more complex) is a pretty reasonable concern to have:
```
if (__builtin_amdgcn_processor_is("gfx900"))
  do_stuff();

bool b = (bool)__builtin_amdgcn_processor_is("gfx900");
if (b)
  do_stuff();
```
because that's not intuitive. What's worse, debugging this when your intuition 
is wrong will be *incredibly* difficult because the behavior in a -O0 build is 
very likely going to do the right thing, so you end up debugging optimized code 
instead.

> Furthermore, is the current formulation particularly novel?

The shape of it isn't novel (a feature test which takes an argument and returns 
something truthy), which is actually why I'm pushing back so hard. This looks 
and feels like `__has_builtin` or other feature testing macros. AFAIK, those 
aren't tied to optimizer behavior so tightly. I think the default expectation 
is that you should be able to query the processor information at any point you 
want, store the results anywhere you want, and use them later with the expected 
semantics; any other behavior starts to feel like a miscompile.

As a counter-example, consider `__has_builtin`:
```
bool has_builtin = __has_builtin(__builtin_whatever);
// ...later...
if (has_builtin)
  __builtin_whatever();
```
if the call to `__has_builtin` returned false and we still ended up calling 
`__builtin_whatever`, users would be baffled as to why. And if it returned 
`true` and we didn't call the builtin, users would be just as baffled. So what 
I'm struggling to understand is why these situations are materially different 
from each other.

https://github.com/llvm/llvm-project/pull/134016
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to