[PATCH] D61243: Check build-type dependent variables for -flto as well.

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 21 12:10:42 PDT 2019


lebedev.ri added a comment.

In D61243#1717083 <https://reviews.llvm.org/D61243#1717083>, @fhahn wrote:

> I just realised that this patch was still around!
>
> In D61243#1484435 <https://reviews.llvm.org/D61243#1484435>, @lebedev.ri wrote:
>
> > In D61243#1484319 <https://reviews.llvm.org/D61243#1484319>, @fhahn wrote:
> >
> > > In D61243#1481819 <https://reviews.llvm.org/D61243#1481819>, @lebedev.ri wrote:
> > >
> > > > Two thoughts:
> > > >
> > > > 1. Why is `LDFLAGS` being set conditionally, is there a warning otherwise?
> > >
> > >
> > > I think it is set conditionally, because the selected linker exe may not support it. When doing LTO  and saving stats, the assumption is that people are using clang for linking.
> >
> >
> > Unless it is intentionally overridden along the way by the user, CMake is always using the compiler as the linker.
>
>
> That makes sense, thanks! But I think moving away from CFLAGS, CXXFLAGS and LDFLAGS is an orthogonal change to this one and there potentially are additional things that can go wrong there. Do you think that and the change as is makes sense?


I don't think the change as-is is bad. Though i'm not sure it will catch everything - it can be set in subdirectory too.
Have you checked what happens if you do `list(APPEND LDFLAGS -save-stats=obj)` unconditionally?


Repository:
  rT test-suite

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

https://reviews.llvm.org/D61243





More information about the llvm-commits mailing list