tejohnson added a comment.
In D108881#2971651 <https://reviews.llvm.org/D108881#2971651>, @tbaeder wrote:
> That code has changed quite a bit since I've worked on it.
>
> The only problem I could see is that passing `-flto=thin -flto` and choosing
> thin LTO kinda makes sense if you interpret `-flto` as just "use LTO". The
> `-flto=thin` is more specific, so it could make sense to pick that. But
> that's just something I wonder while looking at it. Not sure if we process
> other options like this by just choosing the last one.
The default of -flto is documented as -flto=full, so I think the change here
makes sense. In general the last option wins.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4495-4497
+ Arg *A = Args.getLastArg(options::OPT_flto, options::OPT_flto_EQ);
+ if (A && A->getOption().matches(options::OPT_flto))
CmdArgs.push_back("-flto");
----------------
mnadeem wrote:
> Another option would be to do the following, but i am not sure if there is
> any code that explicitly checks for/needs "=full":
>
> ```
> if (D.getLTOMode() == LTOK_Thin)
> CmdArgs.push_back("-flto=thin");
> else
> CmdArgs.push_back("-flto");
> ```
Yes, that should be fine since the default is full. But better yet, make the
else pass the explicit -flto=full.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4496
+ Arg *A = Args.getLastArg(options::OPT_flto, options::OPT_flto_EQ);
+ if (A && A->getOption().matches(options::OPT_flto))
CmdArgs.push_back("-flto");
----------------
Can we consolidate the option handling into parseLTOMode, which does the rest
of the -flto option handling and sets the LTOMode returned by the below call to
getLTOMode()? The default of -flto is "full" so it can be treated there as
such. Actually, looks like it should already be doing this?
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4498
CmdArgs.push_back("-flto");
- else {
- if (D.getLTOMode() == LTOK_Thin)
- CmdArgs.push_back("-flto=thin");
- else
- CmdArgs.push_back("-flto=full");
- }
+ else if (D.getLTOMode() == LTOK_Thin)
+ CmdArgs.push_back("-flto=thin");
----------------
Looks like the result of getLTOMode() is already saved in the LTOMode local
variable.
================
Comment at: clang/test/Driver/lto.c:97
+//
+// FLTO-THIN: -flto=thin
+// FLTO-FULL: -flto=full
----------------
Probably want to check that we don't have additional -flto options being passed
too, here and on the below lines.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108881/new/
https://reviews.llvm.org/D108881
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
