rampitec added inline comments.

================
Comment at: clang/include/clang/Driver/Options.td:3097-3101
+def mtgsplit : Flag<["-"], "mtgsplit">, Group<m_amdgpu_Features_Group>,
+  HelpText<"Enable threadgroup split execution mode (AMDGPU only)">;
+def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group<m_amdgpu_Features_Group>,
+  HelpText<"Disable threadgroup split execution mode (AMDGPU only)">;
+
----------------
kzhuravl wrote:
> kzhuravl wrote:
> > kzhuravl wrote:
> > > rampitec wrote:
> > > > kzhuravl wrote:
> > > > > kzhuravl wrote:
> > > > > > tra wrote:
> > > > > > > We have `BoolFOption` to generate `-fsomething` and 
> > > > > > > `-fno-something`
> > > > > > The reason we use m_amdgpu_Features_Group is it is getting 
> > > > > > transformed to target features (-mtgsplit to +tgsplit, mno-tgsplit 
> > > > > > to -tgsplit. For example, tgsplit target feature in AMDGPU backend:
> > > > > > 
> > > > > > https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/AMDGPU.td#L157
> > > > > > 
> > > > > > Does BoolFOption get translated to target features as well?
> > > > > Quickly glancing over Options.td, BoolFOption is in f_group, and does 
> > > > > not get automatically converted to target-features
> > > > We could probably create similar BoolMOption. This is not only tgsplit, 
> > > > there are plenty of such binary options around.
> > > agreed, seems like a good choice given there is BoolFOption, BoolGOption
> > but it will still need to be in m_amdgpu_Features_Group because 
> > https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/AMDGPU.cpp#L403
> >  unless we want to switch away from that. so maybe make a group an 
> > optional, last template parameter to BoolMOption?
> Other targets also have its own corresponding groups: 
> https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Driver/Options.td#L151
Looks like. It is not just the same thing as BoolFOption or BoolGOption.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to