Page MenuHomePhabricator

[CodeGen] Fix lowering for returning the result of an extractvalue
ClosedPublic

Authored by sunfish on Aug 29 2019, 3:29 PM.

Details

Summary

When the number of return values exceeds the number of registers available, SelectionDAGBuilder::visitRet transforms a function's return to use a pointer to a buffer to hold return values. When the returned value is an operator such as extractvalue, the value may have a non-zero result number. Add that number to the indexing when obtaining the values to store.

This fixes https://bugs.llvm.org/show_bug.cgi?id=43132.

Diff Detail

Repository
rL LLVM

Event Timeline

sunfish created this revision.Aug 29 2019, 3:29 PM
tlively accepted this revision.Aug 29 2019, 8:48 PM

Generally LGTM, although the tests might be more readable if you pass -wasm-disable-explicit-locals and -wasm-keep-registers as well. It's also concerning that no one noticed this issue before. Do you know why other targets may not have run into it?

This revision is now accepted and ready to land.Aug 29 2019, 8:48 PM
sunfish updated this revision to Diff 218020.Aug 29 2019, 9:26 PM
  • Use -wasm-disable-explicit-locals -wasm-keep-registers to make the test more readable

Generally LGTM, although the tests might be more readable if you pass -wasm-disable-explicit-locals and -wasm-keep-registers as well. It's also concerning that no one noticed this issue before. Do you know why other targets may not have run into it?

Returning aggregates in LLVM IR for most C ABIs is a complex dance, where the frontend switches to sret for structs that don't fit in registers. But here we have a Rust type, which doesn't have to follow the C ABI, which gets lowered to an aggregate return in LLVM IR without using sret. And, it turns out that wasm32 allows fewer return values than most other targets -- just one, so the Rust type doesn't fit in registers. So it gets lowered by the backend, which is where it hits this bug.

This revision was automatically updated to reflect the committed changes.