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: > 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. Works for me. We could fold HIP and OpenMP together, both using generic options, and then leave OpenCL to adopt the same if they wish. 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
