qiongsiwu1 marked 3 inline comments as done.
qiongsiwu1 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) {
----------------
MaskRay wrote:
> shchenz wrote:
> > 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.
> If you are going to add `-no-integrated-as=0`, I suggest that you rename the
> option to `-integrated-as` and use `-integrated-as=1` instead.
`-no-integrated-as=0` is no longer added. The backend default will take care of
the case.
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
