This revision was automatically updated to reflect the committed changes.
Closed by commit rL317605: New clang option -fno-plt which avoids the PLT and
lazy binding while making… (authored by tmsriram).
Changed prior to commit:
https://reviews.llvm.org/D39079?vs=119950&id=121947#toc
Repository
rnk added a comment.
I'd also like to highlight that LLVM has many users with different needs.
Sometimes we need to "disagree and commit". Flags are one of the primary ways
that we have to do that.
https://reviews.llvm.org/D39079
___
cfe-commits m
tmsriram added a comment.
Ping. How do we take this forward?
https://reviews.llvm.org/D39079
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
tmsriram added a comment.
In https://reviews.llvm.org/D39079#905519, @joerg wrote:
> In https://reviews.llvm.org/D39079#905468, @rnk wrote:
>
> > In https://reviews.llvm.org/D39079#905454, @joerg wrote:
> >
> > > It also increases the pressure on the branch predictor, so it is not
> > > really b
joerg added a comment.
In https://reviews.llvm.org/D39079#905468, @rnk wrote:
> In https://reviews.llvm.org/D39079#905454, @joerg wrote:
>
> > It also increases the pressure on the branch predictor, so it is not really
> > black and white.
>
>
> I don't understand this objection. I'm assuming th
tmsriram added a comment.
In https://reviews.llvm.org/D39079#905468, @rnk wrote:
> In https://reviews.llvm.org/D39079#905454, @joerg wrote:
>
> > It also increases the pressure on the branch predictor, so it is not really
> > black and white.
>
>
> I don't understand this objection. I'm assuming
rnk added a comment.
In https://reviews.llvm.org/D39079#905454, @joerg wrote:
> It also increases the pressure on the branch predictor, so it is not really
> black and white.
I don't understand this objection. I'm assuming that the PLT stub is an
indirect jump through the PLTGOT, not a hotpat
tmsriram added a comment.
In https://reviews.llvm.org/D39079#905454, @joerg wrote:
> In https://reviews.llvm.org/D39079#905396, @rnk wrote:
>
> > In https://reviews.llvm.org/D39079#905372, @joerg wrote:
> >
> > > Why again is this a good idea?
> >
> >
> > It saves the direct jump to the PLT, redu
tmsriram added a comment.
In https://reviews.llvm.org/D39079#905423, @rnk wrote:
> In https://reviews.llvm.org/D39079#905395, @joerg wrote:
>
> > Let me phrase it differently. What is this patch (and the matching backend
> > PR) supposed to achieve? There are effectively two ways to get rid of P
joerg added a comment.
In https://reviews.llvm.org/D39079#905396, @rnk wrote:
> In https://reviews.llvm.org/D39079#905372, @joerg wrote:
>
> > Why again is this a good idea?
>
>
> It saves the direct jump to the PLT, reducing icache pressure, which is a
> major cost in some workloads.
It also
hfinkel added a comment.
> what is breaking the ELF interposition rules
Frankly, in retrospect, ELF's interposition rules (or at least the defaults for
those rules), seem suboptimal on several fronts. While breaking them has some
unfortunate consistency implications, I don't consider breaking t
rnk added a comment.
In https://reviews.llvm.org/D39079#905395, @joerg wrote:
> Let me phrase it differently. What is this patch (and the matching backend
> PR) supposed to achieve? There are effectively two ways to get rid of PLT
> entries:
> (1) Bind references locally. This is effectively w
On Tue, Oct 24, 2017 at 10:25 AM, Joerg Sonnenberger via Phabricator
wrote:
> joerg added a comment.
>
> Let me phrase it differently. What is this patch (and the matching backend
> PR) supposed to achieve? There are effectively two ways to get rid of PLT
> entries:
> (1) Bind references locally
rnk added a comment.
In https://reviews.llvm.org/D39079#905372, @joerg wrote:
> Why again is this a good idea?
It saves the direct jump to the PLT, reducing icache pressure, which is a major
cost in some workloads.
> This is an even worse hack than -Bsymbolic,
Personally, I would like to bui
joerg added a comment.
Let me phrase it differently. What is this patch (and the matching backend PR)
supposed to achieve? There are effectively two ways to get rid of PLT entries:
(1) Bind references locally. This is effectively what -Bsymbolic does and what
is breaking the ELF interposition ru
hfinkel added a comment.
In https://reviews.llvm.org/D39079#905371, @tmsriram wrote:
> In https://reviews.llvm.org/D39079#905353, @hfinkel wrote:
>
> > Noting that, as @vit9696 pointed out in https://reviews.llvm.org/D38554,
> > this does not suppress uses of the PLT that occur from
> > backen
tmsriram added a comment.
In https://reviews.llvm.org/D39079#905372, @joerg wrote:
> Why again is this a good idea? This is an even worse hack than -Bsymbolic,
> the latter at least is visible in ELF header without code inspection. This is
> breaking core premises of ELF.
Could you elaborate
tmsriram added a comment.
In https://reviews.llvm.org/D39079#905353, @hfinkel wrote:
> Noting that, as @vit9696 pointed out in https://reviews.llvm.org/D38554,
> this does not suppress uses of the PLT that occur from
> backend/optimizer-generated functions (e.g., calls into compiler-rt and
>
joerg added a comment.
Why again is this a good idea? This is an even worse hack than -Bsymbolic, the
latter at least is visible in ELF header without code inspection. This is
breaking core premises of ELF.
https://reviews.llvm.org/D39079
___
cfe-
hfinkel added subscribers: vit9696, hfinkel.
hfinkel added a comment.
Noting that, as @vit9696 pointed out in https://reviews.llvm.org/D38554, this
does not suppress uses of the PLT that occur from backend/optimizer-generated
functions (e.g., calls into compiler-rt and similar).
https://revie
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
https://reviews.llvm.org/D39079
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
tmsriram added inline comments.
Comment at: lib/CodeGen/CGCall.cpp:1859
+if (auto *Fn = dyn_cast(TargetDecl)) {
+ if (!Fn->isDefined() && !AttrOnCallSite) {
+FuncAttrs.addAttribute(llvm::Attribute::NonLazyBind);
rnk wrote:
> Remind me what happen
tmsriram updated this revision to Diff 119950.
tmsriram added a comment.
Added test test/CodeGen/noplt.c
https://reviews.llvm.org/D39079
Files:
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/CGCall.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend
rnk added a comment.
In https://reviews.llvm.org/D39079#904050, @lebedev.ri wrote:
> No tests?
+1, there should be an -emit-llvm test in clang/test/CodeGen/.
Comment at: lib/CodeGen/CGCall.cpp:1859
+if (auto *Fn = dyn_cast(TargetDecl)) {
+ if (!Fn->isDefined() && !A
lebedev.ri added a comment.
No tests?
https://reviews.llvm.org/D39079
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
tmsriram added a comment.
Ping.
https://reviews.llvm.org/D39079
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
tmsriram created this revision.
New clang option -fno-plt which avoids the PLT and lazy binding while making
external calls.
GCC supports -fno-plt,
https://gcc.gnu.org/ml/gcc-patches/2015-05/msg1.html. This patch adds this
to clang which marks all externally defined functions with the "nol
27 matches
Mail list logo
