shchenz added inline comments.

================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:700
+    CmdArgs.push_back(Args.MakeArgString(
+        Twine(PluginOptPrefix) + "-no-integrated-as=" + NoIntegratedAs));
+  else if (!UseIntegratedAs) {
----------------
qiongsiwu1 wrote:
> shchenz wrote:
> > qiongsiwu1 wrote:
> > > shchenz wrote:
> > > > Seems other options leverage the default value in the back end, for 
> > > > example the default value for `DisableIntegratedAS` in backend is 
> > > > false, so when the front end requires integrated-as, maybe we can save 
> > > > the option here?
> > > Ah thanks for the comment! 
> > > 
> > > > maybe we can save the option here?
> > > 
> > > Could you help me understand what we mean by "the option"? Do we mean 
> > > saving (or using?) the value of `-f[no]-integrated-as` explicitly here 
> > > somehow instead of relying on `ToolChain.useIntegratedAs()`? How do we 
> > > intend to use the saved option value? My understanding is that 
> > > `DisableIntegratedAS` takes its value from the option `-no-integrated-as` 
> > > if `-no-integrated-as` is specified. As pointed out eariler, 
> > > `DisableIntegratedAS` is false by default. This code explicitly sets 
> > > `-no-integrated-as` to `0` or `1`, so for the LTO use case, we overwrite 
> > > the back end default since the option is always present. 
> > For example, if front-end requires to use integrated-assembler which is 
> > same with back-end's default value, we don't need to pass 
> > `-no-integrated-as=0`? We only pass the option `-no-integrated-as=1` when 
> > `if (IsOSAIX && !UseIntegratedAs)`.
> Ah I see! Thanks for the clarification! 
> 
> @w2yehia and I discussed this and we preferred setting the option explicitly 
> regardless of the backend's default. The reason was that we did not want to 
> leave a hanging case where the option was not passed to the backend, a case 
> in which we would rely on a non-local option(`DisableIntegratedAS`)'s default 
> value to control the backend. In other words, always passing in the option 
> allowed us to reason about this code locally within this file. @w2yehia feel 
> free to chime in if I am not describing our discussion correctly. 
> 
> Could you help me understand the benefit of not passing in the option for the 
> default case? 
> Could you help me understand the benefit of not passing in the option for the 
> default case?

Too many options pass from front-end to back-end is a reason. And another 
reason is: I met one case that there is a back-end option has no default value, 
so each front-end, like clang and FORTRAN will have to explicitly pass the same 
option. I was asked to set a default in the back-end, so no need to explicitly 
set the options in each front-end.

If what I read is right, some bool type options like `EmulatedTLS`, 
`EnableStackSizeSection` are only passed when their values are not the same 
with the back-end's default.

I am ok to keep it as now if you guys already have an agreement. This is just 
minor I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152924

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

Reply via email to