yaxunl 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 = ---------------- JonChesterfield wrote: > 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. OpenCL options are defined by OpenCL spec. There may be difficulties to alias them to other options. I am OK to alias HIP options to more generic options. 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
