[Wasm RyuJIT] Partially implement genStructReturn#126326
[Wasm RyuJIT] Partially implement genStructReturn#126326kg wants to merge 3 commits intodotnet:mainfrom
Conversation
…t by just generating a return
kg
added
arch-wasm
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull request overview
Adds an initial WebAssembly-specific implementation of CodeGen::genStructReturn to start handling struct returns in the Wasm RyuJIT backend, as part of addressing top asserts seen during wasm crossgen.
Changes:
- Implement partial
genStructReturnhandling forGT_RETURNwhere the return value is aFIELD_LIST. - Consume field list uses to keep liveness consistent, then attempt to return.
| const unsigned regCount = retTypeDesc.GetReturnRegCount(); | ||
|
|
||
| assert(regCount <= MAX_RET_REG_COUNT); | ||
|
|
There was a problem hiding this comment.
actualOp1, retTypeDesc, and regCount are not used outside of assert. In non-DEBUG builds assert compiles out, leaving these locals unused (and potentially failing the build under warnings-as-errors). Either remove them for now or use (void) casts / use regCount in the logic (e.g., compare against the FIELD_LIST use count).
| (void)actualOp1; | |
| (void)retTypeDesc; | |
| (void)regCount; |
|
Can confirm that this fixes: |
kg
changed the title
| // The field list's individual fields should have preceded us in LIR and code to push them onto the stack | ||
| // should already have been generated. We should also only have one field (see MAX_RET_REG_COUNT assert, | ||
| // above.) As a result, all we need to do is generate a return opcode. | ||
| GetEmitter()->emitIns(INS_return); |
There was a problem hiding this comment.
We should not be emitting the return here, it will be part of the epilog.
I think this method can simply be: foreach (field in fieldList) genConsumeReg(field).
Since we don't have multi-regs, I think the NYI below can be removed and we can assert it is always a field list.
There was a problem hiding this comment.
can't a method have multiple returns in it? Do they all turn into branches targeting the epilog?
There was a problem hiding this comment.
Do they all turn into branches targeting the epilog?
No (although we do have an epilog-number-limiting transformation which we should probably disable for WASM). They simply turn into multiple epilogs.
Addresses a top assert from #125756
Scenarios other than fieldlist are not implemented because I don't have a repro for them yet.