Skip to content

Rebaseline code size tests#15461

Merged
kripken merged 4 commits into
mainfrom
update
Nov 9, 2021
Merged

Rebaseline code size tests#15461
kripken merged 4 commits into
mainfrom
update

Conversation

@kripken
Copy link
Copy Markdown
Member

@kripken kripken commented Nov 9, 2021

Ran into the need for this because a reversion PR (#15460) fails unexpectedly.

@kripken kripken requested a review from sbc100 November 9, 2021 16:15
@kripken kripken enabled auto-merge (squash) November 9, 2021 16:19
@kripken
Copy link
Copy Markdown
Member Author

kripken commented Nov 9, 2021

@sbc100 This fails even though I just updated the numbers using emsdk install tot. And I verified the tot has not changed since I created the PR - emsdk install tot locally has nothing new to download. Do I need to do anything else to update these?

@sbc100
Copy link
Copy Markdown
Collaborator

sbc100 commented Nov 9, 2021

@sbc100 This fails even though I just updated the numbers using emsdk install tot. And I verified the tot has not changed since I created the PR - emsdk install tot locally has nothing new to download. Do I need to do anything else to update these?

No, I think that should work...

@kripken
Copy link
Copy Markdown
Member Author

kripken commented Nov 9, 2021

Very strange...

I guess I can update this file manually to the expectation. But I'm worried as that suggests different codegen is happening on my machine versus the bots.

If you have time can you see if rebaselining locally for you agrees with me or with the bots?

@sbc100
Copy link
Copy Markdown
Collaborator

sbc100 commented Nov 9, 2021

Very strange...

I guess I can update this file manually to the expectation. But I'm worried as that suggests different codegen is happening on my machine versus the bots.

If you have time can you see if rebaselining locally for you agrees with me or with the bots?

Sure.

My process is normally to build llvm and binaryen from source at ToT before doing the rebaselining, but I think emsdk tot should work just as well.

@kripken
Copy link
Copy Markdown
Member Author

kripken commented Nov 9, 2021

I've tried building locally, I've tried verifying I get the exact same emsdk hash as CI, but I still get the same difference...

Anyhow, there's clearly something wrong on either my machine or on CI. When you get a chance to compare @sbc100 let's see which results your machine agrees with.

@kripken
Copy link
Copy Markdown
Member Author

kripken commented Nov 9, 2021

It seems like the problem might be that I had newest emscripten+llvm+binaryen, but had not manually run npm install, so I was on an older closure. Hopefully that fixes it...

@kripken kripken merged commit a872af6 into main Nov 9, 2021
@kripken kripken deleted the update branch November 9, 2021 22:30
mmarczell-graphisoft pushed a commit to GRAPHISOFT/emscripten that referenced this pull request Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants