pirama added a comment.
From the discussion, it seems it is theoretically feasible to make optimization
for speed a function-level attribute as well. After looking at the
PassMangerBuilder for this bug, I think that'll make the optimization passes
cleaner by keeping the passes and their behavi
hfinkel added a comment.
In https://reviews.llvm.org/D30920#710329, @mehdi_amini wrote:
> In https://reviews.llvm.org/D30920#710209, @hfinkel wrote:
>
> > Yes, we should do this. I don't understand why this is tricky. Actually, I
> > think having these kinds of decisions explicit in the code of
mehdi_amini added a subscriber: joerg.
mehdi_amini added a comment.
In https://reviews.llvm.org/D30920#710209, @hfinkel wrote:
> Yes, we should do this. I don't understand why this is tricky. Actually, I
> think having these kinds of decisions explicit in the code of the
> transformations would
hfinkel added a comment.
In https://reviews.llvm.org/D30920#703305, @mehdi_amini wrote:
> The fundamental difference, is that Os/Oz especially are treated as
> `optimizations directive` that are independent of the pass pipeline:
> instructing that "loop unroll should not increase size" is indep
mehdi_amini added a comment.
The fundamental difference, is that Os/Oz especially are treated as
`optimizations directive` that are independent of the pass pipeline:
instructing that "loop unroll should not increase size" is independent of
*where* is loop unroll inserted in the pipeline.
The i
hfinkel added a comment.
In https://reviews.llvm.org/D30920#703083, @mehdi_amini wrote:
> In https://reviews.llvm.org/D30920#703082, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D30920#700741, @mehdi_amini wrote:
> >
> > > Yes, the issue is only about how the driver accepts Os for the link e
mehdi_amini added a comment.
In https://reviews.llvm.org/D30920#703082, @hfinkel wrote:
> In https://reviews.llvm.org/D30920#700741, @mehdi_amini wrote:
>
> > Yes, the issue is only about how the driver accepts Os for the link even
> > though it has no effect
> > (O0/https://reviews.llvm.org/ow
hfinkel added a comment.
In https://reviews.llvm.org/D30920#700741, @mehdi_amini wrote:
> In https://reviews.llvm.org/D30920#700574, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D30920#700557, @mehdi_amini wrote:
> >
> > > In https://reviews.llvm.org/D30920#700433, @tejohnson wrote:
> > >
>
mehdi_amini added a comment.
(By the way, this is what is done on Darwin: the -O flag is *ignored* for the
link step invocation, because we couldn't find a "right way" of correctly
honoring it that would be logical to the user in all case).
https://reviews.llvm.org/D30920
__
mehdi_amini added a comment.
It would be reasonable to *not* forward any flag to the linker (plugin), but
not to rewrite them with a different meaning.
https://reviews.llvm.org/D30920
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://l
mehdi_amini added a comment.
In https://reviews.llvm.org/D30920#702979, @pirama wrote:
> The driver (accepts, but) ignores Os and other optimization flags for non-lto
> link-only actions. That it has an effect for LTO is seems to be an
> implementation detail. Since optimization flags are com
pirama added a comment.
In https://reviews.llvm.org/D30920#700741, @mehdi_amini wrote:
> In https://reviews.llvm.org/D30920#700574, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D30920#700557, @mehdi_amini wrote:
> >
> > > In https://reviews.llvm.org/D30920#700433, @tejohnson wrote:
> > >
> >
mehdi_amini added a comment.
In https://reviews.llvm.org/D30920#700574, @hfinkel wrote:
> In https://reviews.llvm.org/D30920#700557, @mehdi_amini wrote:
>
> > In https://reviews.llvm.org/D30920#700433, @tejohnson wrote:
> >
> > > In https://reviews.llvm.org/D30920#700133, @pcc wrote:
> > >
> > >
pirama updated this revision to Diff 91738.
pirama added a comment.
Explicitly pass -O2 instead of relying on the default opt level.
https://reviews.llvm.org/D30920
Files:
lib/Driver/ToolChains/CommonArgs.cpp
test/Driver/gold-lto.c
Index: test/Driver/gold-lto.c
===
hfinkel added a comment.
In https://reviews.llvm.org/D30920#700557, @mehdi_amini wrote:
> In https://reviews.llvm.org/D30920#700433, @tejohnson wrote:
>
> > In https://reviews.llvm.org/D30920#700133, @pcc wrote:
> >
> > > In https://reviews.llvm.org/D30920#700077, @tejohnson wrote:
> > >
> > > >
mehdi_amini added a comment.
In https://reviews.llvm.org/D30920#700433, @tejohnson wrote:
> In https://reviews.llvm.org/D30920#700133, @pcc wrote:
>
> > In https://reviews.llvm.org/D30920#700077, @tejohnson wrote:
> >
> > > Until everything is converted to using size attributes, it seems like a
tejohnson added a comment.
In https://reviews.llvm.org/D30920#700133, @pcc wrote:
> In https://reviews.llvm.org/D30920#700077, @tejohnson wrote:
>
> > Until everything is converted to using size attributes, it seems like a
> > correct fix for the bug is to accept these options in the gold-plugin
pirama marked 3 inline comments as done.
pirama added inline comments.
Comment at: lib/Driver/ToolChains/CommonArgs.cpp:369
if (A->getOption().matches(options::OPT_O4) ||
-A->getOption().matches(options::OPT_Ofast))
+A->getOption().matches(options::OPT_Ofast)
pirama updated this revision to Diff 91671.
pirama added a comment.
Address review comments
https://reviews.llvm.org/D30920
Files:
lib/Driver/ToolChains/CommonArgs.cpp
test/Driver/gold-lto.c
Index: test/Driver/gold-lto.c
===
mehdi_amini added a comment.
Agree with @pcc. Unless anyone has a need to have "perfect" support for Os,
this is the right direction.
https://reviews.llvm.org/D30920
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-
pcc added a comment.
In https://reviews.llvm.org/D30920#700077, @tejohnson wrote:
> Until everything is converted to using size attributes, it seems like a
> correct fix for the bug is to accept these options in the gold-plugin and
> pass through the LTO API to the PassManagerBuilder.
Not nec
tejohnson added a comment.
Interested in pcc's thoughts, as https://bugs.llvm.org/show_bug.cgi?id=32155
mentioned you already discussed with him. Note that some of the passes that
check PassManagerBuilder::sizeLevel are added during the ThinLTO back end (e.g.
populateModulePassManager which che
pirama created this revision.
Herald added a subscriber: mehdi_amini.
Address PR32155: Skip passing -Os and -Oz to the Gold plugin using
-plugin-opt.
https://reviews.llvm.org/D30920
Files:
lib/Driver/ToolChains/CommonArgs.cpp
test/Driver/gold-lto.c
Index: test/Driver/gold-lto.c
==
23 matches
Mail list logo
