[PATCH] D154797: [CUDA][HIP] Rename and fix `-fcuda-approx-transcendentals`

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7228
+  } else {
+    Args.ClaimAllArgs(options::OPT_fgpu_approx_transcendentals);
+    Args.ClaimAllArgs(options::OPT_fno_gpu_approx_transcendentals);
----------------
MaskRay wrote:
> You can use `Args.claimAllArgs(options::OPT_fgpu_approx_transcendentals, options::OPT_fno_gpu_approx_transcendentals);`
will do


================
Comment at: clang/lib/Frontend/InitPreprocessor.cpp:1294
+    if (!LangOpts.HIP)
+      Builder.defineMacro("__CLANG_CUDA_APPROX_TRANSCENDENTALS__");
+    Builder.defineMacro("__CLANG_GPU_APPROX_TRANSCENDENTALS__");
----------------
tra wrote:
> I think we can remove it. I don't think we need to keep the old one around. Internal headers have been changed and the macro was never intended for public use. 
will remove


================
Comment at: clang/test/Driver/hip-options.hip:209
+
+// APPROXNEG-NOT: warning
----------------
MaskRay wrote:
> If `%t` happens to be in a path with `warning` as a substring, this will spuriously fail.
> 
> Suggest `%clang -fdriver-only -Werror... 2>&1 | count 0` to test that there is no warning/error.
will do


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154797