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>
whitequark commented on Issue #1615:
Do I need to do something about the CI?
bjorn3 commented on Issue #1615:
Yes, I don't know why this would change the test output though.
whitequark commented on Issue #1615:
I don't know why this would change the test output though.
Hm, I'm not sure either unfortunately.
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 extendedv1
is split back into twoi32
arguments. Since this legalizes into aniconst.i32 0
the twoi32
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 offn1
is split into twoi32
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 }
bjorn3 commented on Issue #1615:
I'm a little surprised to see it that smart, but that looks like what's happening.
isplit
andiconcat
inside legalizations are immediately applied when possible without creating an instruction and then legalizing them afterwards.
whitequark commented on Issue #1615:
Thanks @iximeow! I've fixed the test.
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.
whitequark commented on Issue #1615:
I fiddled with git some, hope this helps.
Last updated: Dec 23 2024 at 12:05 UTC