Stream: git-wasmtime

Topic: wasmtime / PR #8883 closes #2827: remove srclocs from bug...


view this post on Zulip Wasmtime GitHub notifications bot (Jun 28 2024 at 02:21):

zleyyij requested fitzgen for a review on PR #8883.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 28 2024 at 02:21):

zleyyij requested wasmtime-compiler-reviewers for a review on PR #8883.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 28 2024 at 02:21):

zleyyij opened PR #8883 from zleyyij:main to bytecodealliance:main:

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Prior conversation:
https://github.com/bytecodealliance/wasmtime/issues/2827#issuecomment-2184904073

https://bytecodealliance.zulipchat.com/#narrow/stream/217117-cranelift/topic/Removing.20srclocs.20with.20bugpoint

This change removes sourcelocs from bugpoint output to make the reduced clif IR easier to read.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 28 2024 at 16:41):

fitzgen submitted PR review:

Thanks! One suggestion/question below.

Also cc @bjorn3 who I think has a bunch of experience with bugpoint.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 28 2024 at 16:41):

fitzgen submitted PR review:

Thanks! One suggestion/question below.

Also cc @bjorn3 who I think has a bunch of experience with bugpoint.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 28 2024 at 16:41):

fitzgen created PR review comment:

It isn't clear to me that we should remove them all at once. There might be some panic due to the way we are processing a particular series of srclocs. Can we try removing subset(s) of them incrementally? (I have never touched bugpoint fwiw, so I don't have a sense of the difficulty of what I am asking and if it isn't practical then we can land this as-is).

view this post on Zulip Wasmtime GitHub notifications bot (Jun 28 2024 at 16:51):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 28 2024 at 16:51):

bjorn3 created PR review comment:

I think the approach used by try_resolve_aliases would work here: Try removing all srclocs and see if the bug reproduces. If not revert the change and keep them for the remainder of the run.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 28 2024 at 17:17):

zleyyij updated PR #8883.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 28 2024 at 17:19):

zleyyij commented on PR #8883:

I refactored so that it's done in a manner similar to try_resolve_aliases.

try_resolve_aliases appears to be called twice, I don't know why this is, but I currently only have try_remove_sourcelocs called once.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 28 2024 at 19:10):

zleyyij updated PR #8883.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 28 2024 at 19:24):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 28 2024 at 19:59):

fitzgen submitted PR review:

Thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Jun 28 2024 at 19:59):

fitzgen has enabled auto merge for PR #8883.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 08 2024 at 19:21):

bjorn3 commented on PR #8883:

I think something went wrong here. It still isn't merged after a week.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 08 2024 at 19:24):

fitzgen has disabled auto merge for PR #8883.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 08 2024 at 19:24):

alexcrichton closed without merge PR #8883.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 08 2024 at 19:24):

alexcrichton reopened PR #8883.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 08 2024 at 19:25):

alexcrichton has enabled auto merge for PR #8883.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 08 2024 at 21:14):

alexcrichton merged PR #8883.


Last updated: Nov 22 2024 at 16:03 UTC