schittir added inline comments.
================ Comment at: clang/include/clang/Basic/TargetOptions.h:91 /// \brief Code object version for AMDGPU. - CodeObjectVersionKind CodeObjectVersion; + CodeObjectVersionKind CodeObjectVersion = CodeObjectVersionKind::COV_4; ---------------- aaron.ballman wrote: > tahonermann wrote: > > I see that `COV_4` is the default as specified in > > `clang/include/clang/Driver/Options.td`, but I wonder if we want to > > duplicate that value here. I'm a bit more inclined to default to `COV_None` > > so that we're not trying to maintain the same default in multiple places. I > > don't know that it matters though. > > > > Perhaps it would make sense to add a "default" option value as is done for > > EABI? I'm not sure. > I think `none` makes the most sense to me. This is set via a CC1 option, and > the driver doesn't unconditionally set it > (https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L1073), > so I think `none` is safer than assuming `CodeObjectVersion` is meaningful > when it's not set by -cc1. WDYT? I agree, and that's why I made the change to none. @tahonermann CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146604/new/ https://reviews.llvm.org/D146604 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
