avt77 added a comment.
In https://reviews.llvm.org/D43578#1068417, @rsmith wrote:
> I think we need to fix the overlap issue as a prerequisite to adding timers
> with large amounts of overlap, especially self-overlap. Otherwise the numbers
> will likely do more harm than good, as they will sign
avt77 added a comment.
In https://reviews.llvm.org/D43578#1067995, @rsmith wrote:
> I think https://reviews.llvm.org/D45619 is a good change, and I'd like to see
> that get committed.
Could you give me LGTM in this case? I'm going to publish "recursive"(ovelaped)
timers but I'd like to base i
mzolotukhin added a comment.
> What kinds of would be useful to you? (How do you want the
> runtime of Clang broken down?) Are vertical slices (through all Clang's
> various layers and potentially parts of LLVM) -- as this patch will produce
> -- useful, or would you really want horizontal sli
rsmith added a comment.
In https://reviews.llvm.org/D43578#1068127, @mzolotukhin wrote:
> > Who is the audience for this information?
> > What information do they want from a time report?
> > How do we present that information in a way that's not misleading (given
> > Clang's architecture)?
>
mzolotukhin added a comment.
Hi,
I've been monitoring compile-time for quite a while, so let my put my 2 cents
here too.
> Who is the audience for this information?
> What information do they want from a time report?
> How do we present that information in a way that's not misleading (given
kimgr added a comment.
In https://reviews.llvm.org/D43578#1067974, @rsmith wrote:
> In https://reviews.llvm.org/D43578#1067891, @kimgr wrote:
>
> > I disagreed up until the last paragraph :)
>
>
> Can you say which things you disagree with? There seem to be two important
> facts here:
>
> 1. LLV
rsmith added a comment.
In https://reviews.llvm.org/D43578#1067879, @avt77 wrote:
> In https://reviews.llvm.org/D43578#1067768, @rsmith wrote:
>
> > Last time I looked at doing this, I found that LLVM's timer infrastructure
> > was fundamentally unsuitable for adding timers like these to Clang.
rsmith added a comment.
In https://reviews.llvm.org/D43578#1067891, @kimgr wrote:
> I disagreed up until the last paragraph :)
Can you say which things you disagree with? There seem to be two important
facts here:
1. LLVM's timing infrastructure cannot correctly report timings when one
funct
kimgr added a comment.
Heh, my e-mail response was eaten along the way. Continued here:
On Sat, Apr 14, 2018 at 1:53 AM, Richard Smith - zygoloid via
Phabricator via cfe-commits wrote:
> rsmith added a comment.
>
> So I don't think this patch is reasonable for that reason. I'm also not sure
>
Hi Richard,
On Sat, Apr 14, 2018 at 1:53 AM, Richard Smith - zygoloid via
Phabricator via cfe-commits wrote:
> rsmith added a comment.
>
> So I don't think this patch is reasonable for that reason. I'm also not sure
> whether this, as is, is addressing a pressing use case -- for Clang
> develop
avt77 added a comment.
In https://reviews.llvm.org/D43578#1067768, @rsmith wrote:
> Last time I looked at doing this, I found that LLVM's timer infrastructure
> was fundamentally unsuitable for adding timers like these to Clang.
Thank you for this answer. As I understand we should close both t
rsmith added a comment.
Last time I looked at doing this, I found that LLVM's timer infrastructure was
fundamentally unsuitable for adding timers like these to Clang. The big
problems are that Clang switches between "layers" and pieces that should be
independently accounted at a very large numb
avt77 added a comment.
In https://reviews.llvm.org/D43578#1065827, @davezarzycki wrote:
> It wasn't my suggestion. @thakis wrote: "We probably should have a separate
> bool in clang and key this off that and make -ftime-report set both."
Sorry, but it's done now. Any comments, suggestions, req
davezarzycki added a comment.
It wasn't my suggestion. @thakis wrote: "We probably should have a separate
bool in clang and key this off that and make -ftime-report set both."
https://reviews.llvm.org/D43578
___
cfe-commits mailing list
cfe-commits
avt77 updated this revision to Diff 142198.
avt77 added a reviewer: davezarzycki.
avt77 added a comment.
Herald added a subscriber: mgorny.
I removed the dependence on TimePassesIsEnabled (as @davezarzycki sugested) and
fixed the issue with failed test (tnx to @russell.gallop). As result the patc
avt77 added a comment.
In https://reviews.llvm.org/D43578#1064113, @russell.gallop wrote:
> We also see an assertion failure prior to the revert. At r329738:
Yes, you're right - thaks again for this test case.
Repository:
rL LLVM
https://reviews.llvm.org/D43578
_
On Wed, Apr 11, 2018 at 6:18 AM, Andrew V. Tischenko via Phabricator via
cfe-commits wrote:
> avt77 added a comment.
>
> In https://reviews.llvm.org/D43578#1062950, @thakis wrote:
>
> > @davezarzycki remarks in https://reviews.llvm.org/D45485 that this
> breaks the shared build. The proposed fix
russell.gallop added a comment.
We also see an assertion failure prior to the revert. At r329738:
$ cat test.cpp
template struct A {
template using rebind_alloc = _Other;
};
template struct _Wrap_alloc {
template
using rebind_alloc = typename A<_Alloc>::template rebind_all
avt77 added a comment.
In https://reviews.llvm.org/D43578#1062950, @thakis wrote:
> @davezarzycki remarks in https://reviews.llvm.org/D45485 that this breaks the
> shared build. The proposed fix there is to make several of clang's modules
> depend on LLVM's IR library ("Core"). This seems weird
davezarzycki added a comment.
Hi @efriedma – Sorry about that. I haven't directly used `svn` in years thanks
to the `git svn` bridge.
Repository:
rL LLVM
https://reviews.llvm.org/D43578
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
htt
avt77 added a comment.
In https://reviews.llvm.org/D43578#1063462, @thakis wrote:
> Reverted:
Many thanks for your help!
Repository:
rL LLVM
https://reviews.llvm.org/D43578
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.l
thakis added a comment.
Reverted:
$ svn merge -c -329714 .
- Reverse-merging r329714 into '.':
Atest/Frontend/ftime-report-template-decl.cpp
- Recording mergeinfo for reverse merge of r329714 into '.': U .
$ svn merge -c -329693 .
- Reverse-merging r329693 into '.':
Utest/Frontend
efriedma added a comment.
> while both git and svn have revert subcommands
This is kind of misleading; yes, both git and svn have subcommands named
"revert", but "svn revert" doesn't have the right meaning. You have to use
"svn merge" to revert a committed change.
Repository:
rL LLVM
http
avt77 added a comment.
In https://reviews.llvm.org/D43578#1063365, @davezarzycki wrote:
> A revert in practice just means undoing the changes. For example, while both
> `git` and `svn` have `revert` subcommands, you can also just take the
> original diff/patch, pipe it into `patch -R`, and if t
davezarzycki added a comment.
A revert in practice just means undoing the changes. For example, while both
`git` and `svn` have `revert` subcommands, you can also just take the original
diff/patch, pipe it into `patch -R`, and if there are no conflicts, commit the
result.
Repository:
rL LLV
avt77 reopened this revision.
avt77 added a comment.
This revision is now accepted and ready to land.
Obviously, this patch was not ready for commit that's why I reopen the revison.
But I don't know how to revert the patch from trunk: please, help me.
Repository:
rL LLVM
https://reviews.llvm.
avt77 added a comment.
Could you help me with revert of the committed patch?
I'll do all necessary changes but I don't know how I can revert the patch.
Repository:
rL LLVM
https://reviews.llvm.org/D43578
___
cfe-commits mailing list
cfe-commits@l
rsmith added a comment.
In https://reviews.llvm.org/D43578#1062984, @thakis wrote:
> Also, please add cfe-commits to clang changes. Since this wasn't here and the
> patch wasn't seen by clang folks, since
> ftime-report-template-decl.cppwarnings is still failing on the bots (e.g.
> http://lab.
thakis added a comment.
Also, please add cfe-commits to clang changes. Since this wasn't here and the
patch wasn't seen by clang folks, since ftime-report-template-decl.cppwarnings
is still failing on the bots (e.g.
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-f
29 matches
Mail list logo
