[PATCH] D89076: AMDGPU: Update AMDHSA code object version handling

[Konstantin Zhuravlyov via Phabricator via llvm-commits]

kzhuravl marked 2 inline comments as done.
kzhuravl added inline comments.


================
Comment at: clang/include/clang/Driver/Options.td:2464
 
-def mcode_object_v3 : Flag<["-"], "mcode-object-v3">, Group<m_amdgpu_Features_Group>,
-  HelpText<"Enable code object v3 (AMDGPU only)">;
-def mno_code_object_v3 : Flag<["-"], "mno-code-object-v3">, Group<m_amdgpu_Features_Group>,
-  HelpText<"Disable code object v3 (AMDGPU only)">;
+def mcode_object_v3_legacy : Flag<["-"], "mcode-object-v3">, Group<m_Group>,
+  HelpText<"Legacy option to specify code object v3 (AMDGPU only)">;
----------------
t-tye wrote:
> MaskRay wrote:
> > Does this still need `m_amdgpu_Features_Group`?
> Would it be worth stating that the replacement option is "-mllvm --amdhsa-code-object-version=2" and "-mllvm --amdhsa-code-object-version=3" respectively?
I changed it to m_Group on purpose. This way -target-feature +/-code-object-v3 is not added to cc1, which results in a warning:

'+/-code-object-v3' is not a recognized feature for this target (ignoring feature)

Since we removed this feature from our be.


================
Comment at: clang/include/clang/Driver/Options.td:2464-2467
+def mcode_object_v3_legacy : Flag<["-"], "mcode-object-v3">, Group<m_Group>,
+  HelpText<"Legacy option to specify code object v3 (AMDGPU only)">;
+def mno_code_object_v3_legacy : Flag<["-"], "mno-code-object-v3">, Group<m_Group>,
+  HelpText<"Legacy option to specify code object v2 (AMDGPU only)">;
----------------
kzhuravl wrote:
> t-tye wrote:
> > MaskRay wrote:
> > > Does this still need `m_amdgpu_Features_Group`?
> > Would it be worth stating that the replacement option is "-mllvm --amdhsa-code-object-version=2" and "-mllvm --amdhsa-code-object-version=3" respectively?
> I changed it to m_Group on purpose. This way -target-feature +/-code-object-v3 is not added to cc1, which results in a warning:
> 
> '+/-code-object-v3' is not a recognized feature for this target (ignoring feature)
> 
> Since we removed this feature from our be.
Replacement option is already stated in the deprecated warning, in case this option is used. Do we really want to put the same suggestion here? I quickly glanced through ClangCommandLineReference.rst and did not see similar cases.


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

https://reviews.llvm.org/D89076