aaron.ballman added a comment.

It's not clear to me why existing facilities shouldn't be extended to cover 
this case rather than coming up with another feature testing macro. There's 
already plenty of confusion for users to decide between `__has_feature` and 
`__has_extension`, and now we're talking about adding a third choice to the mix.



================
Comment at: clang/docs/LanguageExtensions.rst:260
 
+``__has_target_feature``
+------------------------
----------------
The first question that comes to mind for me is: why is `__has_feature` not 
sufficient?


================
Comment at: clang/docs/LanguageExtensions.rst:275
+  // On amdgcn target
+  #if __has_target_feature("s-memtime-inst")
+    x = __builtin_amdgcn_s_memtime();
----------------
yaxunl wrote:
> tra wrote:
> > Do you have a better example? This particular case could've been handled by 
> > existing `__has_builtin()`.
> > 
> > While I could see usefulness of checking features (e.g. for CUDA/NVPTX it 
> > could be used to chose inline assembly supported only by newer PTX 
> > versions, but even then that choice could be made using existing methods, 
> > even if they are not always direct (e.g. by using CUDA_VERSION as a proxy 
> > for "new enough PTX version").
> > 
> `__has_builtin` returns 1 as long as the builtin is known to clang, even if 
> the builtin is not supported by the target CPU. This is because the required 
> target feature for a builtin is in ASTContext, whereas `__has_builtin` is 
> evaluated in preprocessor, where the information is not known.
> `__has_builtin` returns 1 as long as the builtin is known to clang, even if 
> the builtin is not supported by the target CPU. This is because the required 
> target feature for a builtin is in ASTContext, whereas `__has_builtin` is 
> evaluated in preprocessor, where the information is not known.

Why is that not a deficiency with `__has_builtin` that we should fix?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125555/new/

https://reviews.llvm.org/D125555

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to