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

[sunfish]

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.

[tlively]

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?

[sunfish]

• Use -wasm-disable-explicit-locals -wasm-keep-registers to make the test more readable

[sunfish]

In D66978#1652070, @tlively wrote:

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.

llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

if (!FuncInfo.CanLowerReturn) {
     unsigned NumValues = ValueVTs.size();
 
     SmallVector<SDValue, 4> Chains(NumValues);
     for (unsigned i = 0; i != NumValues; ++i) {
       // An aggregate return value cannot wrap around the address space, so
       // offsets to its parts don't wrap either.
       SDValue Ptr = DAG.getObjectPtrOffset(getCurSDLoc(), RetPtr, Offsets[i]);
 
-      SDValue Val = RetOp.getValue(i);
+      SDValue Val = RetOp.getValue(RetOp.getResNo() + i);
       if (MemVTs[i] != ValueVTs[i])
         Val = DAG.getPtrExtOrTrunc(Val, getCurSDLoc(), MemVTs[i]);
       Chains[i] = DAG.getStore(Chain, getCurSDLoc(), Val,
           // FIXME: better loc info would be nice.
           Ptr, MachinePointerInfo::getUnknownStack(DAG.getMachineFunction()));
     }
 
     Chain = DAG.getNode(ISD::TokenFactor, getCurSDLoc(),

llvm/trunk/test/CodeGen/WebAssembly/multi-return.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -asm-verbose=false -wasm-disable-explicit-locals -wasm-keep-registers | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+; Return multiple values, some of which will be legalized into multiple values.
+declare { i64, i128, i192, i128, i64 } @return_multi_multi()
+
+; Test returning a single value from @return_multi_multi.
+
+define i64 @test0() {
+; CHECK-LABEL: test0
+; CHECK: call     return_multi_multi
+; CHECK: i64.load $0=, 8($1)
+; CHECK: local.copy $push8=, $0
+  %t0 = call { i64, i128, i192, i128, i64 } @return_multi_multi()
+  %t1 = extractvalue { i64, i128, i192, i128, i64 } %t0, 0
+  ret i64 %t1
+}
+
+define i128 @test1() {
+; CHECK-LABEL: test1
+; CHECK: call     return_multi_multi
+; CHECK: i64.load $1=, 16($2)
+; CHECK: i32.const  $push0=, 24
+; CHECK: i32.add  $push1=, $2, $pop0
+; CHECK: i64.load $push2=, 0($pop1)
+; CHECK: i64.store  8($0), $pop2
+; CHECK: i64.store  0($0), $1
+  %t0 = call { i64, i128, i192, i128, i64 } @return_multi_multi()
+  %t1 = extractvalue { i64, i128, i192, i128, i64 } %t0, 1
+  ret i128 %t1
+}
+
+define i192 @test2() {
+; CHECK-LABEL: test2
+; CHECK: call     return_multi_multi
+; CHECK: i32.const  $push0=, 40
+; CHECK: i32.add  $push1=, $3, $pop0
+; CHECK: i64.load $1=, 0($pop1)
+; CHECK: i64.load $2=, 32($3)
+; CHECK: i32.const  $push2=, 48
+; CHECK: i32.add  $push3=, $3, $pop2
+; CHECK: i64.load $push4=, 0($pop3)
+; CHECK: i64.store  16($0), $pop4
+; CHECK: i64.store  0($0), $2
+; CHECK: i64.store  8($0), $1
+  %t0 = call { i64, i128, i192, i128, i64 } @return_multi_multi()
+  %t1 = extractvalue { i64, i128, i192, i128, i64 } %t0, 2
+  ret i192 %t1
+}
+
+define i128 @test3() {
+; CHECK-LABEL: test3
+; CHECK: call     return_multi_multi
+; CHECK: i64.load $1=, 56($2)
+; CHECK: i32.const  $push0=, 64
+; CHECK: i32.add  $push1=, $2, $pop0
+; CHECK: i64.load $push2=, 0($pop1)
+; CHECK: i64.store  8($0), $pop2
+; CHECK: i64.store  0($0), $1
+  %t0 = call { i64, i128, i192, i128, i64 } @return_multi_multi()
+  %t1 = extractvalue { i64, i128, i192, i128, i64 } %t0, 3
+  ret i128 %t1
+}
+
+define i64 @test4() {
+; CHECK-LABEL: test4
+; CHECK: call     return_multi_multi
+; CHECK: i64.load $0=, 72($1)
+; CHECK: local.copy $push8=, $0
+  %t0 = call { i64, i128, i192, i128, i64 } @return_multi_multi()
+  %t1 = extractvalue { i64, i128, i192, i128, i64 } %t0, 4
+  ret i64 %t1
+}
+
+; Test returning multiple values from @return_multi_multi.
+
+define { i64, i128 } @test5() {
+; CHECK-LABEL: test5
+; CHECK: call     return_multi_multi
+; CHECK: i32.const  $push10=, 8
+; CHECK: i32.add  $push11=, $3, $pop10
+; CHECK: i32.const  $push0=, 16
+; CHECK: i32.add  $push1=, $pop11, $pop0
+; CHECK: i64.load $1=, 0($pop1)
+; CHECK: i64.load $2=, 8($3)
+; CHECK: i64.load $push2=, 16($3)
+; CHECK: i64.store  8($0), $pop2
+; CHECK: i32.const  $push12=, 16
+; CHECK: i32.add  $push3=, $0, $pop12
+; CHECK: i64.store  0($pop3), $1
+; CHECK: i64.store  0($0), $2
+  %t0 = call { i64, i128, i192, i128, i64 } @return_multi_multi()
+  %r0 = extractvalue { i64, i128, i192, i128, i64 } %t0, 0
+  %r1 = extractvalue { i64, i128, i192, i128, i64 } %t0, 1
+  %s0 = insertvalue { i64, i128 } undef, i64 %r0, 0
+  %s1 = insertvalue { i64, i128 } %s0, i128 %r1, 1
+  ret { i64, i128 } %s1
+}
+
+define { i128, i128 } @test6() {
+; CHECK-LABEL: test6
+; CHECK: call     return_multi_multi
+; CHECK: i32.const  $push0=, 24
+; CHECK: i32.add  $push1=, $4, $pop0
+; CHECK: i64.load $1=, 0($pop1)
+; CHECK: i32.const  $push2=, 64
+; CHECK: i32.add  $push3=, $4, $pop2
+; CHECK: i64.load $2=, 0($pop3)
+; CHECK: i64.load $3=, 16($4)
+; CHECK: i64.load $push4=, 56($4)
+; CHECK: i64.store  16($0), $pop4
+; CHECK: i32.const  $push5=, 24
+; CHECK: i32.add  $push6=, $0, $pop5
+; CHECK: i64.store  0($pop6), $2
+; CHECK: i64.store  0($0), $3
+; CHECK: i64.store  8($0), $1
+  %t0 = call { i64, i128, i192, i128, i64 } @return_multi_multi()
+  %r1 = extractvalue { i64, i128, i192, i128, i64 } %t0, 1
+  %r3 = extractvalue { i64, i128, i192, i128, i64 } %t0, 3
+  %s0 = insertvalue { i128, i128 } undef, i128 %r1, 0
+  %s1 = insertvalue { i128, i128 } %s0, i128 %r3, 1
+  ret { i128, i128 } %s1
+}
+
+define { i64, i192 } @test7() {
+; CHECK-LABEL: test7
+; CHECK: call     return_multi_multi
+; CHECK: i32.const  $push2=, 40
+; CHECK: i32.add  $push3=, $4, $pop2
+; CHECK: i64.load $1=, 0($pop3)
+; CHECK: i64.load $2=, 8($4)
+; CHECK: i64.load $3=, 32($4)
+; CHECK: i32.const  $push0=, 24
+; CHECK: i32.add  $push1=, $0, $pop0
+; CHECK: i32.const  $push4=, 48
+; CHECK: i32.add  $push5=, $4, $pop4
+; CHECK: i64.load $push6=, 0($pop5)
+; CHECK: i64.store  0($pop1), $pop6
+; CHECK: i64.store  8($0), $3
+; CHECK: i32.const  $push7=, 16
+; CHECK: i32.add  $push8=, $0, $pop7
+; CHECK: i64.store  0($pop8), $1
+; CHECK: i64.store  0($0), $2
+  %t0 = call { i64, i128, i192, i128, i64 } @return_multi_multi()
+  %r0 = extractvalue { i64, i128, i192, i128, i64 } %t0, 0
+  %r2 = extractvalue { i64, i128, i192, i128, i64 } %t0, 2
+  %s0 = insertvalue { i64, i192 } undef, i64 %r0, 0
+  %s1 = insertvalue { i64, i192 } %s0, i192 %r2, 1
+  ret { i64, i192 } %s1
+}
+
+define { i128, i192, i128, i64 } @test8() {
+; CHECK-LABEL: test8
+; CHECK: call     return_multi_multi
+; CHECK: i32.const  $push0=, 64
+; CHECK: i32.add  $push1=, $8, $pop0
+; CHECK: i64.load $1=, 0($pop1)
+; CHECK: i32.const  $push20=, 8
+; CHECK: i32.add  $push21=, $8, $pop20
+; CHECK: i32.const  $push2=, 32
+; CHECK: i32.add  $push3=, $pop21, $pop2
+; CHECK: i64.load $2=, 0($pop3)
+; CHECK: i32.const  $push4=, 48
+; CHECK: i32.add  $push5=, $8, $pop4
+; CHECK: i64.load $3=, 0($pop5)
+; CHECK: i32.const  $push6=, 24
+; CHECK: i32.add  $push7=, $8, $pop6
+; CHECK: i64.load $4=, 0($pop7)
+; CHECK: i64.load $5=, 8($8)
+; CHECK: i64.load $6=, 56($8)
+; CHECK: i64.load $7=, 32($8)
+; CHECK: i64.load $push8=, 16($8)
+; CHECK: i64.store  40($0), $pop8
+; CHECK: i32.const  $push9=, 48
+; CHECK: i32.add  $push10=, $0, $pop9
+; CHECK: i64.store  0($pop10), $4
+; CHECK: i32.const  $push22=, 32
+; CHECK: i32.add  $push11=, $0, $pop22
+; CHECK: i64.store  0($pop11), $3
+; CHECK: i64.store  16($0), $7
+; CHECK: i32.const  $push12=, 24
+; CHECK: i32.add  $push13=, $0, $pop12
+; CHECK: i64.store  0($pop13), $2
+; CHECK: i64.store  0($0), $6
+; CHECK: i64.store  8($0), $1
+; CHECK: i64.store  56($0), $5
+  %t0 = call { i64, i128, i192, i128, i64 } @return_multi_multi()
+  %r0 = extractvalue { i64, i128, i192, i128, i64 } %t0, 0
+  %r1 = extractvalue { i64, i128, i192, i128, i64 } %t0, 1
+  %r2 = extractvalue { i64, i128, i192, i128, i64 } %t0, 2
+  %r3 = extractvalue { i64, i128, i192, i128, i64 } %t0, 3
+  %s0 = insertvalue { i128, i192, i128, i64 } undef, i128 %r3, 0
+  %s1 = insertvalue { i128, i192, i128, i64 } %s0, i192 %r2, 1
+  %s2 = insertvalue { i128, i192, i128, i64 } %s1, i128 %r1, 2
+  %s3 = insertvalue { i128, i192, i128, i64 } %s2, i64 %r0, 3
+  ret { i128, i192, i128, i64 } %s3
+}