junbuml abandoned this revision.
junbuml added a comment.
Abandon this based the last comment in http://reviews.llvm.org/D15289.
http://reviews.llvm.org/D13304
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mai
junbuml added a comment.
Just ping to see if there is any objection about adding the extra check for
CallSites in EHRs in inliner. I will be happy to hear any opinion, suggestion,
or objection.
http://reviews.llvm.org/D13304
___
cfe-commits maili
junbuml added a comment.
If we want to add a check for CallSites in EHRs in inliner, we may be able to
borrow things done in BranchProbabilityInfo::calcColdCallHeuristics, but for
exception handing intrinsics, not for cold, and make getInlineThreshold()
return a lower threshold so that we can
manmanren added a subscriber: manmanren.
manmanren added a comment.
Inliner currently does not include analysis passes such as BPI and BFI. With
the new pass manager, we should be able to hook up inliner with BFI (and we can
handle throw in BFI). Before that, maybe we can add this as part of inl
junbuml added a comment.
Thanks Richard for your comment !
If the frond-end is not a good to place for this, I think there are two places
we can consider : inliner or prune-eh.
1. In inliner, we can directly check if a CallSite branches an exception
region, and then make getInlineThreshold() r
rsmith added a comment.
I am not convinced that it's reasonable to put inlining heuristics into clang's
IR generation. This will cause maintenance problems for the inliner in the
future (anyone tuning the inlining heuristics and thresholds will need to be
aware of this, and clang will behave di
mcrosier added a comment.
@chandlerc: Adding Chandler in case he has an opinion on how to move forward or
how we could go about tuning the cold threshold.
http://reviews.llvm.org/D13304
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http:/
junbuml added a comment.
The basic idea of this change is to avoid inlining callsites invoked in
exception handling regions (EHR) so that we can reduce code size blow-up in
very cold regions and indirectly increase inline opportunities for functions
containing exception handling code.
I think
junbuml added a comment.
I see what you mean. Now, I doubt if
BranchProbabilityInfo::calcColdCallHeuristics() set the Cold before inliner.
As far as I check, BranchProbabilityInfo is executed after inliner.
Another issue is that even if we can add the Cold in callsites in exception
handling
hfinkel added a comment.
In http://reviews.llvm.org/D13304#276660, @junbuml wrote:
> Did you mean to add the Cold in the exception handling region itself instead
> of callsite in throw statements ?
We already have BranchProbabilityInfo::calcColdCallHeuristics, and so adding it
to the callsite
junbuml added a comment.
Did you mean to add the Cold in the exception handling region itself instead of
callsite in throw statements ?
http://reviews.llvm.org/D13304
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi
hfinkel added a comment.
In http://reviews.llvm.org/D13304#269049, @junbuml wrote:
> I just want to ping one more time to see if there is any objection about the
> basic idea of this change. If the basic idea itself is acceptable, then I
> want to find the best way to get idea in.
>
> Please le
junbuml added a comment.
I just want to ping one more time to see if there is any objection about the
basic idea of this change. If the basic idea itself is acceptable, then I want
to find the best way to get idea in.
Please let me know any new suggestion or any opinion about moving this
imple
junbuml added a comment.
Just want to hear if there is any opinion about moving this implementation back
to PrunceEH.cpp like http://reviews.llvm.org/D12979 and adding the minimum
callee size check to avoid marking noinlines on trivial constrictor calls.
Please let me know any objection or sugg
Yes, these are functionally equivalent. However, the difference between
these two is that the call to f() in 1st is in sub-expression of throw
statement, while the call in 2nd is not.
If the call is in the sub-expression of throw statements, we can guarantee
that the call is only invoked to be thr
On Thu, Oct 8, 2015 at 9:54 AM, wrote:
> > I think this actually makes it less general. You would presumably perform
> > different inlining for:
> >
> > throw f(x, y);
> >
> > versus
> >
> > auto k = f(x, y);
> > throw k;
>
> We need to differentiate between these two. For the second case,
> I think this actually makes it less general. You would presumably perform
> different inlining for:
>
> throw f(x, y);
>
> versus
>
> auto k = f(x, y);
> throw k;
We need to differentiate between these two. For the second case, we should
not add any attribute because itÂ’s not invoked in th
On Wed, Oct 7, 2015 at 3:10 PM, Jun Bum Lim via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> junbuml added a comment.
>
> Thanks Richard for the comment.
>
> Initially, I intended to implement this in inliner by checking if a
> callsite is in exception handling regions. However, I decided no
junbuml added a comment.
Thanks Richard for the comment.
Initially, I intended to implement this in inliner by checking if a callsite is
in exception handling regions. However, I decided not to implement this in
inliner because this kind of check should be performed for all callsites if we
imp
rsmith added a subscriber: rsmith.
rsmith added a comment.
This seems like something that's much better handled by the inliner. We should
presumably treat all the code that leads up to the throw as being cold, not
just the invocation of the throw-expression itself, and it seems like it should
b
junbuml added a comment.
Is there any comment on this change? Note that with change, I observed about 6%
performance improvement in spec2006/xalancbmk. I will address any comment to
get this in.
http://reviews.llvm.org/D13304
___
cfe-commits maili
junbuml updated this revision to Diff 36551.
junbuml added a comment.
Just minor cleaning. Please let me know any comment.
http://reviews.llvm.org/D13304
Files:
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGException.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGe
mcrosier added a comment.
FWIW, a llvm based solution was discussed in http://reviews.llvm.org/D12979,
but the clang solution is obviously more robust and easier to implement.
Comment at: lib/CodeGen/CodeGenFunction.h:287
@@ +286,3 @@
+ // True if the current insertion point i
junbuml created this revision.
junbuml added reviewers: hfinkel, mcrosier, reames, ashutosh.nema, majnemer.
junbuml added subscribers: gberry, cfe-commits.
It might be reasonably to avoid inlining CallSites invoked in
exception handling context so that we can reduce code size blow-up in
EH regions
24 matches
Mail list logo
