aaron.ballman 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;
 
----------------
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?


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

Reply via email to