Stream: git-wasmtime

Topic: wasmtime / Issue #1615 Legalize [su]extend.i64 to iconst/...

Wasmtime GitHub notifications bot (Apr 28 2020 at 05:49):

github-actions[bot] commented on Issue #1615:

Subscribe to Label Action

cc @bnjbvr

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:meta"

Thus the following users have been cc'd because of the following labels:

• bnjbvr: cranelift

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Apr 28 2020 at 12:09):

whitequark commented on Issue #1615:

Do I need to do something about the CI?

Wasmtime GitHub notifications bot (Apr 28 2020 at 12:27):

bjorn3 commented on Issue #1615:

Yes, I don't know why this would change the test output though.

Wasmtime GitHub notifications bot (Apr 28 2020 at 13:39):

whitequark commented on Issue #1615:

I don't know why this would change the test output though.

Hm, I'm not sure either unfortunately.

Wasmtime GitHub notifications bot (Apr 28 2020 at 20:46):

iximeow commented on Issue #1615:

The test change is consistent with this patch, the uextend.i64 in the test case legalizes differently:

; over in filetests/filetests/isa/riscv/legalize-abi.clif:

function %split_call_arg(i32) {
    fn1 = %foo(i64)
    fn2 = %foo(i32, i64)
block0(v0: i32):
    v1 = uextend.i64 v0
    call fn1(v1)
    ; check: $(v1l=$V), $(v1h=$V) = isplit v1
    ; check: call fn1($v1l, $v1h)
    call fn2(v0, v1)
    ; check: call fn2(v0, $V, $V)
    return
}

The check that fails is $(v1l=$V), $(v1h=$V) = isplit v1, looking to see that the extended v1 is split back into two i32 arguments. Since this legalizes into an iconst.i32 0 the two i32 values are passed directly instead of going through a concat/split. I'm a little surprised to see it that smart, but that looks like what's happening.

Since the spirit of this test looks to be that the i64 argument of fn1 is split into two i32 that the platform can use, just changing the check will do:

function %split_call_arg(i32) {
    fn1 = %foo(i64)
    fn2 = %foo(i32, i64)
block0(v0: i32):
    v1 = uextend.i64 v0
    call fn1(v1)
    ; check: $(v1h=$V) = iconst.i32 0
    ; check: call fn1(v0, $v1h)
    call fn2(v0, v1)
    ; check: call fn2(v0, $V, $V)
    return
}

Wasmtime GitHub notifications bot (Apr 29 2020 at 09:37):

bjorn3 commented on Issue #1615:

I'm a little surprised to see it that smart, but that looks like what's happening.

isplit and iconcat inside legalizations are immediately applied when possible without creating an instruction and then legalizing them afterwards.

Wasmtime GitHub notifications bot (May 02 2020 at 04:48):

whitequark commented on Issue #1615:

Thanks @iximeow! I've fixed the test.

Wasmtime GitHub notifications bot (May 05 2020 at 21:05):

iximeow commented on Issue #1615:

I noticed on Sunday or so that macos CI for 2d6c977 failed with what looked like a transient error (downloading the rust toolchain timed out or something along those lines), and tried rerunning it then, only to forget to revisit shortly thereafter. It looks like that rerun failed, believing that commit no longer exists?

git checkout --progress --force 2d6c977a4b9061c7a94d324dbaa0a9a93ba383cd
##[error]fatal: reference is not a tree: 2d6c977a4b9061c7a94d324dbaa0a9a93ba383cd
##[error]Git checkout failed with exit code: 128
##[error]Exit code 1 returned from process: file name '/home/runner/runners/2.169.1/bin/Runner.PluginHost', arguments 'action "GitHub.Runner.Plugins.Repository.v1_0.CheckoutTask, Runner.Plugins"'.

I just reran it again to the same failure. The last time I'd seen this category of failure was last week when GitHub was having issues a week or two ago, but that doesn't seem to be the case here. I don't have the faintest clue why CI no longer believes this commit exists: it obviously does exist.

Wasmtime GitHub notifications bot (May 05 2020 at 21:58):

whitequark commented on Issue #1615:

I fiddled with git some, hope this helps.

Last updated: Nov 22 2024 at 16:03 UTC