JonChesterfield added inline comments.

================
Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:879-889
+  // TODO: There are way too many flags that change this. Do we need to check
+  // them all?
+  bool DAZ = DriverArgs.hasArg(options::OPT_cl_denorms_are_zero) ||
+             getDefaultDenormsAreZeroForTarget(Kind);
+  bool FiniteOnly = DriverArgs.hasArg(options::OPT_cl_finite_math_only);
+
+  bool UnsafeMathOpt =
----------------
yaxunl wrote:
> I agree that we'd better refactor this part as a common function used by 
> HIP/OpenMP/OpenCL. However I disagree to let HIP use OpenCL options. HIP uses 
> clang generic options or HIP/CUDA shared options to control these flags. I 
> think OpenMP may consider using similar options as HIP instead of OpenCL 
> options.
It's a mixture. Some flags look generic, some have hip in the name. My 
preference would be to have a generic named flag for each one, and alias 
hip/opencp/cuda onto the generic one.

Aliasing the flags would mean what works today continues working while 
simplifying the control flow in clang. Keeping flags with different names that 
do the same thing is certainly possible but doesn't seem a feature. It makes 
clang more complicated in order to make user build scripts more complicated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105981

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

Reply via email to