================
@@ -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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits