Stream: t-compiler/meetings

Topic: [weekly meeting] 2020-06-25 #54818


Santiago Pastorino (Jun 24 2020 at 21:40, on Zulip):

Hi @T-compiler/meeting; the triage meeting will be starting in 16 hours 20 minutes

Santiago Pastorino (Jun 24 2020 at 21:40, on Zulip):

The @WG-prioritization have done pre-triage in #t-compiler/wg-prioritization

Santiago Pastorino (Jun 24 2020 at 21:41, on Zulip):

@WG-prioritization have prepared the meeting agenda

Santiago Pastorino (Jun 24 2020 at 21:41, on Zulip):

We will have checkins from @WG-rls2.0 and @WG-self-profile

Santiago Pastorino (Jun 24 2020 at 21:42, on Zulip):

@matklad do you have something you want to share about @WG-rls2.0?

Santiago Pastorino (Jun 24 2020 at 21:42, on Zulip):

@Wesley Wiser do you have something you want to share about @WG-self-profile?

Wesley Wiser (Jun 24 2020 at 21:43, on Zulip):

Sure, yeah. I'll write some stuff up.

matklad (Jun 24 2020 at 21:43, on Zulip):

+1

Santiago Pastorino (Jun 25 2020 at 12:09, on Zulip):

@Wesley Wiser @matklad remember that you can paste your checkins directly here https://hackmd.io/jK5av9hMS4aOF6_0cEU9jw?both#WG-checkins

Santiago Pastorino (Jun 25 2020 at 13:04, on Zulip):

Hi @T-compiler/meeting; the triage meeting will be starting in ~ 56 minutes

pnkfelix (Jun 25 2020 at 14:00, on Zulip):

is there a steering meeting tomorrow? /me looks

pnkfelix (Jun 25 2020 at 14:00, on Zulip):

i'll add an announcement bullet for it

Santiago Pastorino (Jun 25 2020 at 14:01, on Zulip):

ohh right

pnkfelix (Jun 25 2020 at 14:02, on Zulip):

Hi @T-compiler/meeting! Add a :wave: emoji to show you're here :)

pnkfelix (Jun 25 2020 at 14:02, on Zulip):

we will start off with 5 minutes for ...

Announcements

pnkfelix (Jun 25 2020 at 14:03, on Zulip):
pnkfelix (Jun 25 2020 at 14:03, on Zulip):
pnkfelix (Jun 25 2020 at 14:03, on Zulip):

hmm, that's a lot of issues to digest in one chunk

pnkfelix (Jun 25 2020 at 14:05, on Zulip):

(@nikomatsakis and @Santiago Pastorino and I are privately discussing things to add to this template and so I am thinking harder at the moment about its structure)

Santiago Pastorino (Jun 25 2020 at 14:06, on Zulip):

after the meeting we can keep discussing this, lately seems like the agenda is very packed

pnkfelix (Jun 25 2020 at 14:06, on Zulip):

I don't remember if we started this last week or not, but we are going to be doing WG checkins at the start of the meeting instead of the end

pnkfelix (Jun 25 2020 at 14:06, on Zulip):

so

pnkfelix (Jun 25 2020 at 14:07, on Zulip):

(assuming no one else wants to chime in with separate announcements)

pnkfelix (Jun 25 2020 at 14:07, on Zulip):

WG checkins

@WG-rls2.0 checkin by @matklad:

RFC2912 progress:

Org progress:

Impl progress:

matklad (Jun 25 2020 at 14:07, on Zulip):

:wave:

pnkfelix (Jun 25 2020 at 14:08, on Zulip):

Very happy to see that @Jonas Schievink joined rust-analyzer team

pnkfelix (Jun 25 2020 at 14:08, on Zulip):

or however you want to phrase it

Jonas Schievink (Jun 25 2020 at 14:08, on Zulip):

:wave:

pnkfelix (Jun 25 2020 at 14:08, on Zulip):

@WG-self-profile checkin by @Wesley Wiser:

pnkfelix (Jun 25 2020 at 14:10, on Zulip):

@Wesley Wiser is the LLVM blame really all just going to LLVM on whole crate? I would think that with codegen unit partitioning, we could at least assign blame to ... a subset of the crate that corresponds to the DefId's for a cgu?

nikomatsakis (Jun 25 2020 at 14:10, on Zulip):

Yes, plus, can we perhaps apportion it proportionately to the amount of LLVM bit code?

pnkfelix (Jun 25 2020 at 14:10, on Zulip):

Or is it just not useful to show such cgu-based blame, because our cgu contents are,I think, scattered?

nikomatsakis (Jun 25 2020 at 14:10, on Zulip):

It's not accurate, but I bet it's a reasonable approximation

pnkfelix (Jun 25 2020 at 14:11, on Zulip):

@nikomatsakis is that going to account for work from LLVM's inlining choices?

nikomatsakis (Jun 25 2020 at 14:11, on Zulip):

I would measure it before optimization

pnkfelix (Jun 25 2020 at 14:11, on Zulip):

but that's my point?

pnkfelix (Jun 25 2020 at 14:11, on Zulip):

a fn may have small bitcode before inlining decisions

nikomatsakis (Jun 25 2020 at 14:12, on Zulip):

OK? and probably editing that fn isn't going to help you as much as editing the callees :)

pnkfelix (Jun 25 2020 at 14:12, on Zulip):

but after LLVM decides to inline, its proportionate compile-time may increase to more than you'd expect based on original bitcode size?

simulacrum (Jun 25 2020 at 14:12, on Zulip):

we do have full callgraph information, right? so we could plausibly show a "transitive cost" or whatever

nikomatsakis (Jun 25 2020 at 14:12, on Zulip):

anyway I don't know, it's a heuristic, we'd have to see how it works

Wesley Wiser (Jun 25 2020 at 14:12, on Zulip):

(Sorry, afk right now. Will respond at the end of the meeting)

pnkfelix (Jun 25 2020 at 14:12, on Zulip):

anyway it sounds cool, and worth talking about more. :smile:

simulacrum (Jun 25 2020 at 14:12, on Zulip):

(well modulo LLVM inlining dynamic dispatch, but there's not much we can do there)

nikomatsakis (Jun 25 2020 at 14:12, on Zulip):

anyway this is all great news Wesley Wiser -- I'm sure we can experiment and improve the LLVM efficacy

pnkfelix (Jun 25 2020 at 14:13, on Zulip):

/me tries to remember if we can extract knowledge of inlining decisions in some manner

pnkfelix (Jun 25 2020 at 14:13, on Zulip):

anyway like @Santiago Pastorino said, agenda is packed, so lets move along

pnkfelix (Jun 25 2020 at 14:14, on Zulip):

s

T-compiler

pnkfelix (Jun 25 2020 at 14:14, on Zulip):

arg

pnkfelix (Jun 25 2020 at 14:14, on Zulip):

Beta-nominations

T-compiler

pnkfelix (Jun 25 2020 at 14:14, on Zulip):
pnkfelix (Jun 25 2020 at 14:16, on Zulip):

I have mixed feelings

pnkfelix (Jun 25 2020 at 14:17, on Zulip):

given that @Alex Crichton themself said that this is using a relatively new Cargo feature, I'm somewhat inclined to let it ride the trains

nikomatsakis (Jun 25 2020 at 14:17, on Zulip):

still

nikomatsakis (Jun 25 2020 at 14:18, on Zulip):

the patch is pretty trivial :)

Santiago Pastorino (Jun 25 2020 at 14:18, on Zulip):

I can confirm the Firefox linking error mentioned in #73136 (comment) does not happen anymore with nightly-2020-06-20, which is the first nightly with this fix. I cannot, however, say anything about nightly-2020-06-19, because rustc crashed during compilation (and so did nightly-2020-06-18).

nikomatsakis (Jun 25 2020 at 14:18, on Zulip):

I'm not sure which cargo feature this is leaning on exactly?

nikomatsakis (Jun 25 2020 at 14:18, on Zulip):

I think I'd be inclined to backpor it if it fixes a FF regression

pnkfelix (Jun 25 2020 at 14:18, on Zulip):

profile overrides

Santiago Pastorino (Jun 25 2020 at 14:18, on Zulip):

FWIW this patch as-is uses a relatively new Cargo feature of profile overrides, and if it's backported I'd recommend double-checking that it works before landing.

nikomatsakis (Jun 25 2020 at 14:19, on Zulip):

I see

Esteban Küber (Jun 25 2020 at 14:20, on Zulip):

Will cargo need to backport anything?

nikomatsakis (Jun 25 2020 at 14:20, on Zulip):

yeah I guess that is an important question to resolve

simulacrum (Jun 25 2020 at 14:20, on Zulip):

no cargo backport needed, there's an alternative fix

pnkfelix (Jun 25 2020 at 14:20, on Zulip):

@simulacrum as in, a different PR you'd backport to beta?

pnkfelix (Jun 25 2020 at 14:20, on Zulip):

that's a different conversation, no?

simulacrum (Jun 25 2020 at 14:21, on Zulip):

well no, basically we can declaratively say that compiler-builtins gets this special arg

simulacrum (Jun 25 2020 at 14:21, on Zulip):

or we can specify so in bootstrap manually

simulacrum (Jun 25 2020 at 14:21, on Zulip):

obviously the cargo.toml change is nicer, but they should be equivalent

simulacrum (Jun 25 2020 at 14:21, on Zulip):

(I consider them equivalent, modulo things like xargo, but those usually use nightly toolchains I think)

pnkfelix (Jun 25 2020 at 14:22, on Zulip):

I think I'd 1. prefer to consider the change @simulacrum is described, but 2. I'd want to actually see it too before I approve it.

simulacrum (Jun 25 2020 at 14:22, on Zulip):

I mean it's a similar 2-3 line change in bootstrap, but sure

pnkfelix (Jun 25 2020 at 14:23, on Zulip):

I'm just saying, I cannot infer what the change is from this conversation.

simulacrum (Jun 25 2020 at 14:23, on Zulip):

basically https://github.com/rust-lang/rust/commit/926c7b6bb53b8a30b712759eaa9641040d15caef

pnkfelix (Jun 25 2020 at 14:24, on Zulip):

where did that come from? I don't see it linked from the PR ?

simulacrum (Jun 25 2020 at 14:24, on Zulip):

(as a policy note, I would personally consider this basically the equivalent of "merge conflicts" when backporting and something that T-compiler doesn't need to approve explicitly/separately, but maybe that's not the right approach)

simulacrum (Jun 25 2020 at 14:24, on Zulip):

that's an earlier version of this PR

pnkfelix (Jun 25 2020 at 14:25, on Zulip):

ah I see

simulacrum (Jun 25 2020 at 14:25, on Zulip):

https://github.com/rust-lang/rust/pull/73136#issuecomment-641537423

nikomatsakis (Jun 25 2020 at 14:25, on Zulip):

I'd be :+1: to backporting that version, based on the fact we know it fixes things

nikomatsakis (Jun 25 2020 at 14:25, on Zulip):

and it sure seems cleaner

nikomatsakis (Jun 25 2020 at 14:25, on Zulip):

to just declare a lot of CGUs vs having some special-cased logic

nikomatsakis (Jun 25 2020 at 14:26, on Zulip):

"cleaner" == "less risky"

pnkfelix (Jun 25 2020 at 14:26, on Zulip):

@nikomatsakis you're saying both versions of this PR are cleaner than teh status quo, right?

pnkfelix (Jun 25 2020 at 14:26, on Zulip):

I do agree with that

pnkfelix (Jun 25 2020 at 14:27, on Zulip):

and I also would be more inclined to :thumbs_up: backporting the earlier version, for the same reason that @nikomatsakis outlined (we know it fixes the FF problem, and it doesn't rely on new cargo stuff)

pnkfelix (Jun 25 2020 at 14:27, on Zulip):

so, okay, lets do that then?

pnkfelix (Jun 25 2020 at 14:27, on Zulip):

"Reoder order in which MinGW libs are linked to fix recent breakage" #73184

- "Recent upstream mingw-w64 changes made libmsvcrt depend on libmingwex breaking compilation in some cases when using external MinGW."
- This one was discussed last week. We decided to wait a week.
- According to @Vadim Petrochenkov "CI has old mingw and this PR passed it."

pnkfelix (Jun 25 2020 at 14:28, on Zulip):

(I think based on @Vadim Petrochenkov 's comment, we can go ahead and approve this. I was tempted to unilaterally approve it last week after I saw that remark, but decided waiting wouldn't hurt.)

pnkfelix (Jun 25 2020 at 14:29, on Zulip):
Santiago Pastorino (Jun 25 2020 at 14:29, on Zulip):

it's not merged on master

Santiago Pastorino (Jun 25 2020 at 14:29, on Zulip):

maybe I should just not add those in the future

pnkfelix (Jun 25 2020 at 14:29, on Zulip):

@Santiago Pastorino hmm

pnkfelix (Jun 25 2020 at 14:30, on Zulip):

I don't think its fundamentally wrong to put up the backport question before the merge to master occurs

Santiago Pastorino (Jun 25 2020 at 14:30, on Zulip):

right now @WG-prioritization is adding all the PRs that have the beta/stable nomination label regardless if they are merged or not

pnkfelix (Jun 25 2020 at 14:30, on Zulip):

the only thing I would question is doing the actual backport before the merge to master

Santiago Pastorino (Jun 25 2020 at 14:30, on Zulip):

I wound't :), I guess after some discussions in the PR the code may change

pnkfelix (Jun 25 2020 at 14:30, on Zulip):

You wouldn't do the backport? Or you wouldn't question it?

Santiago Pastorino (Jun 25 2020 at 14:31, on Zulip):

I wouldn't backport, unless we have the release in this week or something like that and we need to rush things

pnkfelix (Jun 25 2020 at 14:31, on Zulip):

Santiago Pastorino said:

I wound't :), I guess after some discussions in the PR the code may change

yes, but I tend to trust the reviewers to re-nominate a PR if they think the code has changed so much as to warrant re-approval

Santiago Pastorino (Jun 25 2020 at 14:31, on Zulip):

fair enough :)

pnkfelix (Jun 25 2020 at 14:32, on Zulip):

I still am not sure whether we are discussing the question of when one lands the backport, vs when one approves a backport at this meeting, vs when one decides to put a backport nomination onto this meetings agenda

pnkfelix (Jun 25 2020 at 14:33, on Zulip):

but we have gone way off course

nikomatsakis (Jun 25 2020 at 14:33, on Zulip):

I'm not sure I like the fix (I'd sort of rather not have the duplicates to begin with) but it doesn't seem harmful

pnkfelix (Jun 25 2020 at 14:33, on Zulip):

(which is fine; but lets get back on track)

pnkfelix (Jun 25 2020 at 14:35, on Zulip):

nikomatsakis said:

I'm not sure I like the fix (I'd sort of rather not have the duplicates to begin with) but it doesn't seem harmful

so there's a semi-analogy here with our diagnostics

pnkfelix (Jun 25 2020 at 14:35, on Zulip):

namely, we de-duplicate diagnostics that are presented to the end-user

pnkfelix (Jun 25 2020 at 14:35, on Zulip):

however, certain compilation modes will disable that deduplication

pnkfelix (Jun 25 2020 at 14:36, on Zulip):

in particular, compiletest will not deduplicate

pnkfelix (Jun 25 2020 at 14:36, on Zulip):

(I cannot recall offhand if this is because of the json error-format, or some other flag it passes. I think its a different flag, probably a -Z one. I encountered this while prototyping cargo report-future-incompat.)

Esteban Küber (Jun 25 2020 at 14:37, on Zulip):

Regarding the fix, I don't like it either but it is what we were doing before

nikomatsakis (Jun 25 2020 at 14:37, on Zulip):

well

pnkfelix (Jun 25 2020 at 14:37, on Zulip):

The reason I bring this up is: @nikomatsakis , would you be happier if we had a way to opt into not removing the duplicates, and then using that in our test suite ?

nikomatsakis (Jun 25 2020 at 14:37, on Zulip):

no

nikomatsakis (Jun 25 2020 at 14:37, on Zulip):

this seems different,

nikomatsakis (Jun 25 2020 at 14:37, on Zulip):

in that it'll "change behavior" versus producing more errors

pnkfelix (Jun 25 2020 at 14:37, on Zulip):

(I mean for the tests where we currently have no duplicates, that is. To ensure they don't regress)

pnkfelix (Jun 25 2020 at 14:38, on Zulip):

true

nikomatsakis (Jun 25 2020 at 14:38, on Zulip):

Esteban Küber said:

Regarding the fix, I don't like it either but it is what we were doing before

before the original PR?

Esteban Küber (Jun 25 2020 at 14:38, on Zulip):

Correct

nikomatsakis (Jun 25 2020 at 14:38, on Zulip):

OK.

nikomatsakis (Jun 25 2020 at 14:38, on Zulip):

well I voted for :back: in any case, but waiting till it lands seems fine

pnkfelix (Jun 25 2020 at 14:39, on Zulip):

okay, beta-backport approved. I'll try to make sure my backport approving comment captures some of the desiderata outlined here.

Esteban Küber (Jun 25 2020 at 14:39, on Zulip):

If you want I can add a fixme comment to it. And I don't know if eddyb is avail for the review. If someone here wants to look at the pr yourself... :blush:

pnkfelix (Jun 25 2020 at 14:39, on Zulip):

libs-impl

pnkfelix (Jun 25 2020 at 14:40, on Zulip):

T-rustdoc

nikomatsakis (Jun 25 2020 at 14:40, on Zulip):

Esteban Küber said:

If you want I can add a fixme comment to it. And I don't know if eddyb is avail for the review. If someone here wants to look at the pr yourself... :blush:

you can r? me

pnkfelix (Jun 25 2020 at 14:41, on Zulip):

/me wonders whether he/we made a mistake in agreeing to make decisions here regarding rustdoc

pnkfelix (Jun 25 2020 at 14:41, on Zulip):

i guess backport approved

pnkfelix (Jun 25 2020 at 14:42, on Zulip):

Stable-nominations

T-compiler

pnkfelix (Jun 25 2020 at 14:42, on Zulip):

we were just discussing this, obviously

nikomatsakis (Jun 25 2020 at 14:42, on Zulip):

heh I don't .. really understand this diff in #73644. Am I being silly? Isn't if(A && B && C) { X } the same as if(A) { if (B&&C) { X } }?

Santiago Pastorino (Jun 25 2020 at 14:42, on Zulip):

note that it's the one from @Esteban Küber ... right :)

nikomatsakis (Jun 25 2020 at 14:43, on Zulip):

oh, I see, the for changes too

pnkfelix (Jun 25 2020 at 14:44, on Zulip):

the body of the for, you mean, right?

pnkfelix (Jun 25 2020 at 14:44, on Zulip):

oh no

Santiago Pastorino (Jun 25 2020 at 14:44, on Zulip):

nikomatsakis said:

heh I don't .. really understand this diff in #73644. Am I being silly? Isn't if(A && B && C) { X } the same as if(A) { if (B&&C) { X } }?

the else matches the top if and not the whole condition

pnkfelix (Jun 25 2020 at 14:44, on Zulip):

the boundary conditions too

nikomatsakis (Jun 25 2020 at 14:45, on Zulip):

I didn't notice there was an else, ok

pnkfelix (Jun 25 2020 at 14:45, on Zulip):

so back to the stable-nom of #73485

pnkfelix (Jun 25 2020 at 14:46, on Zulip):

I guess @nikomatsakis already marked :back: on it, and I'm not expecting objections from anyone else, I think

pnkfelix (Jun 25 2020 at 14:46, on Zulip):

so stable backport approved then

pnkfelix (Jun 25 2020 at 14:46, on Zulip):

remaining stable noms

libs-impl

T-rustdoc

pnkfelix (Jun 25 2020 at 14:47, on Zulip):

PRs S-waiting-on-team

T-compiler

pnkfelix (Jun 25 2020 at 14:47, on Zulip):

it seems like the author really wants a second

nikomatsakis (Jun 25 2020 at 14:48, on Zulip):

Thing is, I can't tell if we want to do this :)

Santiago Pastorino (Jun 25 2020 at 14:48, on Zulip):

literally they requested a second to second :)

pnkfelix (Jun 25 2020 at 14:48, on Zulip):

right

pnkfelix (Jun 25 2020 at 14:48, on Zulip):

the question of "is this the right thing for cargo" is interesting

pnkfelix (Jun 25 2020 at 14:48, on Zulip):

because

nikomatsakis (Jun 25 2020 at 14:49, on Zulip):

I guess I'm inclined to go for it, though, based on the fact that they had good experiences, with some caveats.

pnkfelix (Jun 25 2020 at 14:49, on Zulip):

due to cargo's tight integration with rustc

pnkfelix (Jun 25 2020 at 14:49, on Zulip):

I figure we can resolve that design in a lazy fashion

pnkfelix (Jun 25 2020 at 14:49, on Zulip):

so the real question to me is

pnkfelix (Jun 25 2020 at 14:49, on Zulip):

Does this resolve problems for non-cargo rustc drivers

nikomatsakis (Jun 25 2020 at 14:49, on Zulip):

I also think that the cargo concern (many possible consumers) isn't (to me) a total blocker

nikomatsakis (Jun 25 2020 at 14:49, on Zulip):

e.g., we give warnings now about unused imports and things

nikomatsakis (Jun 25 2020 at 14:49, on Zulip):

even though feature flags might mean it's wrong

nikomatsakis (Jun 25 2020 at 14:50, on Zulip):

I personally would opt into this

nikomatsakis (Jun 25 2020 at 14:50, on Zulip):

oh I guess you can't easily #[allow]...

nikomatsakis (Jun 25 2020 at 14:50, on Zulip):

...well, maybe add dummy uses, idk.

pnkfelix (Jun 25 2020 at 14:50, on Zulip):

anyway

pnkfelix (Jun 25 2020 at 14:50, on Zulip):

we don't need to make a decision about this in this meeting

pnkfelix (Jun 25 2020 at 14:50, on Zulip):

just raise awareness

nikomatsakis (Jun 25 2020 at 14:51, on Zulip):

but anyway I'd like to hear from petrochenkov and others who were more actively participating. I guess I'll put in review the thread and put in some thoughts.

pnkfelix (Jun 25 2020 at 14:51, on Zulip):

Is there anyone present here who is actively interested in non-cargo rustc drivers?

pnkfelix (Jun 25 2020 at 14:51, on Zulip):

I think that we may want/need more representation from that community

pnkfelix (Jun 25 2020 at 14:51, on Zulip):

or maybe I have the wrong instinct about whose voice to promote

pnkfelix (Jun 25 2020 at 14:52, on Zulip):

lets move along

pnkfelix (Jun 25 2020 at 14:52, on Zulip):

Issues of Note

Short Summary

pnkfelix (Jun 25 2020 at 14:52, on Zulip):

P-critical

T-compiler

pnkfelix (Jun 25 2020 at 14:52, on Zulip):
pnkfelix (Jun 25 2020 at 14:53, on Zulip):
pnkfelix (Jun 25 2020 at 14:53, on Zulip):

okay, this is all much less scary than I feared when I first saw that summary

pnkfelix (Jun 25 2020 at 14:53, on Zulip):

libs-impl

T-rustdoc

pnkfelix (Jun 25 2020 at 14:53, on Zulip):

Unassigned P-high regressions

Beta regressions

pnkfelix (Jun 25 2020 at 14:54, on Zulip):

Nightly regressions

pnkfelix (Jun 25 2020 at 14:54, on Zulip):

Performance logs

Triage done by njn.
Latest revision: 6bb3dbfc6c6d8992d08431f320ba296a0c2f7498.

Lots of improvements this week! :tada:

Having done this for a few weeks now, I see that close to half of the PRs with significant performance effects are rollups.

Regressions

Improvements

pnkfelix (Jun 25 2020 at 14:54, on Zulip):

I'd love to spend time delving into these victories

pnkfelix (Jun 25 2020 at 14:55, on Zulip):

but we've got only five minutes left

pnkfelix (Jun 25 2020 at 14:55, on Zulip):

and some nominated issues

pnkfelix (Jun 25 2020 at 14:55, on Zulip):

Nominated Issues

T-compiler

pnkfelix (Jun 25 2020 at 14:55, on Zulip):

this is old

pnkfelix (Jun 25 2020 at 14:56, on Zulip):

@Santiago Pastorino why did you nominate?

Santiago Pastorino (Jun 25 2020 at 14:56, on Zulip):

just skip it

pnkfelix (Jun 25 2020 at 14:56, on Zulip):

ooookay

pnkfelix (Jun 25 2020 at 14:57, on Zulip):
Santiago Pastorino (Jun 25 2020 at 14:57, on Zulip):

it's fine there are more important things to discuss

nikomatsakis (Jun 25 2020 at 14:57, on Zulip):

I don't think #72012 is nominated for this team

pnkfelix (Jun 25 2020 at 14:57, on Zulip):

we discussed #72012 at a recent lang team meeting

pnkfelix (Jun 25 2020 at 14:58, on Zulip):

the nomination comment did allude to a question of "how is this behavior implemented in the compiler"

pnkfelix (Jun 25 2020 at 14:58, on Zulip):

so it wasn't absurd to put the issue on agenda

pnkfelix (Jun 25 2020 at 14:59, on Zulip):

but having said that, I'll move along

pnkfelix (Jun 25 2020 at 14:59, on Zulip):
pnkfelix (Jun 25 2020 at 15:00, on Zulip):

is this appropriate for the Windows area group?

pnkfelix (Jun 25 2020 at 15:00, on Zulip):

or do we think it might be a problem with lld clients in general?

pnkfelix (Jun 25 2020 at 15:01, on Zulip):

might be good to check if this happens on lld on Linux

pnkfelix (Jun 25 2020 at 15:01, on Zulip):
pnkfelix (Jun 25 2020 at 15:02, on Zulip):

nomination here is probably outdated, since @Vadim Petrochenkov posted a fix I think

pnkfelix (Jun 25 2020 at 15:02, on Zulip):
nikomatsakis (Jun 25 2020 at 15:03, on Zulip):

I think relnotes makes sense

Santiago Pastorino (Jun 25 2020 at 15:04, on Zulip):

we can do that and close the issue then

nikomatsakis (Jun 25 2020 at 15:04, on Zulip):

but I think we should track LLVM

pnkfelix (Jun 25 2020 at 15:04, on Zulip):

as in, when LLVM decides to make a change like this, we follow suit?

nikomatsakis (Jun 25 2020 at 15:05, on Zulip):

yeah I wouldn't want to like ove-rule and change the behavior

pnkfelix (Jun 25 2020 at 15:06, on Zulip):

floating point arithmetic is so much fun

pnkfelix (Jun 25 2020 at 15:06, on Zulip):

anyway, that's our hour (plus a little more time I stole from you all, sorry!)

pnkfelix (Jun 25 2020 at 15:06, on Zulip):

thanks to everyone in @T-compiler/meeting for attending!

pnkfelix (Jun 25 2020 at 15:07, on Zulip):

stay safe everyone :mask:

Wesley Wiser (Jun 25 2020 at 15:07, on Zulip):

For anyone interested:
To circle back to the self-profile code blame thing: there is actually support for tracing LLVM with the self-profiler but it requires -Znew-pass-manager=y. See https://github.com/rust-lang/rust/pull/68406

Right now, we do know what cgu is being built but the self-profiler doesn't know what's in that CGU. I was hoping the new LLVM pass manager would be getting enabled by default soon-ish which would be a lot cleaner that trying to decide how to handle LLVM time at the CGU level.

I'm personally skeptical that this tool will be that useful if there's such inexact tracing of time spent.

pnkfelix (Jun 25 2020 at 15:08, on Zulip):

@Wesley Wiser you had a prototype PR at one point that made a CGU per-fn, right? If one coupled that with your code blame prototype, then the results could be precise, right?

Wesley Wiser (Jun 25 2020 at 15:09, on Zulip):

Yeah it did roughly that

pnkfelix (Jun 25 2020 at 15:09, on Zulip):

(Obviously there would still be the problem that you'd be inspecting the behavior of CGU per-fn, which may not accurately model the actual costs in the normal system.)

Wesley Wiser (Jun 25 2020 at 15:10, on Zulip):

That's going to affect compilation time pretty drastically though so it's not necessarily going to be representative of where time is being spent normally.

pnkfelix (Jun 25 2020 at 15:10, on Zulip):

Your code blame prototype did roughly that? Like it put in the CGU per-fn to get good results?

Wesley Wiser (Jun 25 2020 at 15:10, on Zulip):

No, sorry.

Wesley Wiser (Jun 25 2020 at 15:10, on Zulip):

The code blame prototype is entirely separate from rustc. It uses the standard -Zself-profile infrastructure.

Wesley Wiser (Jun 25 2020 at 15:11, on Zulip):

Currently, I don't have any changes at all for rustc

Wesley Wiser (Jun 25 2020 at 15:11, on Zulip):

But I will need to make a few small ones to get better reporting.

pnkfelix (Jun 25 2020 at 15:11, on Zulip):

Okay. I was just trying to interpret what you meant by "Yeah it did roughly that"

Wesley Wiser (Jun 25 2020 at 15:11, on Zulip):

The issue is that the self-profiler just sees a huge block of LLVM time because we don't have tracing normally available into that.

Wesley Wiser (Jun 25 2020 at 15:11, on Zulip):

I thought you just meant that prototype PR I had for the CGU meeting.

pnkfelix (Jun 25 2020 at 15:12, on Zulip):

I see, you were just confirming what that prototype did (namely, made "roughly" a CGU per-fn) ?

Wesley Wiser (Jun 25 2020 at 15:12, on Zulip):

Yeah

Wesley Wiser (Jun 25 2020 at 15:12, on Zulip):

Sorry, that was confusing.

pnkfelix (Jun 25 2020 at 15:12, on Zulip):

no problem

andjo403 (Jun 25 2020 at 15:31, on Zulip):

to test if we get something useful we can try this old commit that adds the llvm time-trace to rust selfprofile for the old llvm pass manager https://github.com/andjo403/rust/commit/003d17a5d1a8156daf82588726501a489eb2b10b the question is how we map the mangled names that llvm traces to the DeFids

andjo403 (Jun 25 2020 at 15:32, on Zulip):

@Wesley Wiser I can rebase it if you think that it is somthing that you want to try

Wesley Wiser (Jun 25 2020 at 15:34, on Zulip):

Oooh that's cool

andjo403 (Jun 25 2020 at 15:34, on Zulip):

there is a small change to the llvm code do not know if it is possible to upstream that

Wesley Wiser (Jun 25 2020 at 15:35, on Zulip):

Yeah, I'd hate to have extra patches we have to carry around just to support that.

Wesley Wiser (Jun 25 2020 at 15:35, on Zulip):

But I guess I'm also unsure what's happening with new-pass-manager

Wesley Wiser (Jun 25 2020 at 15:35, on Zulip):

I don't think it's enabled by default yet?

andjo403 (Jun 25 2020 at 15:36, on Zulip):

no the new-pass-manager is not default and it feels like it have halted again was some talking about it for the 9.0 release but have not seen that much after that

andjo403 (Jun 25 2020 at 15:41, on Zulip):

but thinks that it can be interesting even to try with the new-pass-manager as it fells like a better estimate of time then to try to count the llvm-ir that rustc produces

Last update: Nov 25 2020 at 02:30UTC