s390x: Fixes to tailcall and array parameter fix (#49466)

### Small s390x fixes
- Add another tailcall disqualifying condition
- Minor formatting change

### New Array Parameter fix

We have been seeing msbuild tasks fail due to an OverflowException being raised in `(wrapper_alloc)_object:AllocVector` because the number of elements appears to be negative (0xffffffff00000088).

However, this is just the result of a missing type conversion somehow. The original C# code is simply:
```
_metadataStringTable = new string[_reader.ReadInt32()];
```
The IL corresponding to that line reads:
```
converting (in B2: stack: 2) IL_0021: callvirt 0x0a00035e
cmethod = int System.IO.BinaryReader:ReadInt32 ()
Call requires: 1 parameters
converting (in B2: stack: 2) IL_0026: newarr 0x010000b6
```
Note that the return from the `ReadInt32` call is of type `int` (i.e. 32 bits) and is passed unmodified as argument to `newarr`. This remains the same in the initial stages of Mono JIT compilation:
```
call_membase R43 <- [R45 + 0xe8] [int System.IO.BinaryReader:ReadInt32 ()] [s390_r2 <- R44] clobbers: c
il_seq_point il: 0x26, nonempty-stack
newarr R46 <- R43
```
But later the `newarr` is implemented in terms of a function call:
```
call_membase R43 <- [R45 + 0xe8] [int System.IO.BinaryReader:ReadInt32 ()] [s390_r2 <- R44] clobbers: c
il_seq_point il: 0x26, nonempty-stack
i8const R81 <- [2929315620864]
move R83 <- R81
move R84 <- R43
call R46 <- [(wrapper alloc) object object:AllocVector (intptr,intptr)] [s390_r2 <- R83] [s390_r3 <- R84] clobbers: c
```
And here the argument is of type `intptr` (i.e. 64 bits). But the return value of ReadInt32 is still passed through directly to `AllocVector` without any conversion, and this remains true in the final assembler code:
```
b2: 0d e1             basr %r14,%r1 // ReadInt32
b4: b9 04 00 32       lgr %r3,%r2 // <<- simple move of the return value
b8: c0 28 00 00 02 aa iihf %r2,682
be: c0 29 08 d1 28 00 iilf %r2,147924992
c4: c0 e8 00 00 03 ff iihf %r14,1023
ca: c0 e9 9d ec 97 08 iilf %r14,2649528072
d0: 0d ee             basr %r14,%r14 // AllocVector
```
This becomes a problem because the implementation of `AllocVector` does indeed expect a 64-bit input, and throws an exception if that is negative.

In addition, the implementation of `ReadInt32` only computes a 32-bit result in the low 32 bits of the register, and leaves the upper half undefined. Due to the particular code sequence we get a return with the upper half nonzero even though the lower half is a positive integer:
```
e6: e3 20 f0 e0 00 14  lgf %r2,224(%r15) <<- if this is a 32-bit value with the top bit set, then the upper half of %r2 is now 0xffffffff
ec: e3 20 f0 b8 00 50  sty %r2,184(%r15)
f2: b9 04 00 32        lgr %r3,%r2 <<- and so is %r3
f6: c0 01 00 ff 00 ff  lgfi %r0,16711935
fc: b9 e4 00 43        ngrk %r4,%r3,%r0
100: b9 14 00 24       lgfr %r2,%r4
104: 88 20 00 08       srl %r2,8
108: 89 40 00 18       sll %r4,24
10c: b9 e6 40 22       ogrk %r2,%r2,%r4
110: c0 01 ff 00 ff 00 lgfi %r0,-16711936 <<-- the upper half of %r0 is also 0xffffffff
116: b9 e4 00 43       ngrk %r4,%r3,%r0 <<-- and so is %r4
11a: b9 14 00 34       lgfr %r3,%r4
11e: 89 30 00 08       sll %r3,8
122: 88 40 00 18       srl %r4,24
126: b9 e6 40 33       ogrk %r3,%r3,%r4 <<- and now %r3
12a: b9 08 00 23       agr %r2,%r3 <<- and therefore %r2, where the upper half for 0 before
12e: e3 20 f0 b8 00 50 sty %r2,184(%r15)
134: a7 fb 00 f8       aghi %r15,248
138: eb 6e f0 30 00 04 lmg %r6,%r14,48(%r15)
13e: 07 fe             br %r14 <<- and here it is returned unchanged
```

diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c
index ed3c96e..8814851 100644 (file)
--- a/src/mono/mono/mini/method-to-ir.c
+++ b/src/mono/mono/mini/method-to-ir.c
@@ -9798,7 +9798,8 @@ field_access_end:
 
                        context_used = mini_class_check_context_used (cfg, klass);
 
-                       if (sp [0]->type == STACK_I8 || (TARGET_SIZEOF_VOID_P == 8 && sp [0]->type == STACK_PTR)) {
+#ifndef TARGET_S390X
+                       if (sp [0]->type == STACK_I8 && TARGET_SIZEOF_VOID_P == 4) {
                                MONO_INST_NEW (cfg, ins, OP_LCONV_TO_OVF_U4);
                                ins->sreg1 = sp [0]->dreg;
                                ins->type = STACK_I4;
@@ -9806,6 +9807,18 @@ field_access_end:
                                MONO_ADD_INS (cfg->cbb, ins);
                                *sp = mono_decompose_opcode (cfg, ins);
                        }
+#else
+                       /* The array allocator expects a 64-bit input, and we cannot rely
+                          on the high bits of a 32-bit result, so we have to extend.  */
+                       if (sp [0]->type == STACK_I4 && TARGET_SIZEOF_VOID_P == 8) {
+                               MONO_INST_NEW (cfg, ins, OP_ICONV_TO_I8);
+                               ins->sreg1 = sp [0]->dreg;
+                               ins->type = STACK_I8;
+                               ins->dreg = alloc_ireg (cfg);
+                               MONO_ADD_INS (cfg->cbb, ins);
+                               *sp = mono_decompose_opcode (cfg, ins);
+                       }
+#endif
 
                        if (context_used) {
                                MonoInst *args [3];

diff --git a/src/mono/mono/mini/mini-s390x.c b/src/mono/mono/mini/mini-s390x.c
index 835e948..1cc8f83 100644 (file)
--- a/src/mono/mono/mini/mini-s390x.c
+++ b/src/mono/mono/mini/mini-s390x.c
@@ -3471,7 +3471,7 @@ mono_arch_output_basic_block (MonoCompile *cfg, MonoBasicBlock *bb)
                        if (!cfg->r4fp)
                                s390_ledbr (code, ins->dreg, ins->sreg1);
                        break;
-        case OP_TLS_GET: {
+               case OP_TLS_GET: {
                        if (s390_is_imm16 (ins->inst_offset)) {
                                s390_lghi (code, s390_r13, ins->inst_offset);
                        } else if (s390_is_imm32 (ins->inst_offset)) {
@@ -5349,8 +5349,7 @@ mono_arch_register_lowlevel_calls (void)
  */
 
 void
-mono_arch_patch_code_new (MonoCompile *cfg,
-                                                 guint8 *code, MonoJumpInfo *ji, gpointer target)
+mono_arch_patch_code_new (MonoCompile *cfg, guint8 *code, MonoJumpInfo *ji, gpointer target)
 {
        unsigned char *ip = ji->ip.i + code;
        gint64 displace;
@@ -6956,13 +6955,13 @@ mono_arch_tailcall_supported (MonoCompile *cfg, MonoMethodSignature *caller_sig,
                        res = FALSE;
                        break;
                case RegTypeStructByAddr :
-                       if (ainfo[i].reg == STK_BASE) 
+                       if (ainfo[i].reg == STK_BASE || ainfo[i].reg == S390_LAST_ARG_REG)
                                res = FALSE;
                        else
                                res = TRUE;
                        break;
                case RegTypeStructByVal :
-                       if (ainfo[i].reg == STK_BASE) 
+                       if (ainfo[i].reg == STK_BASE || ainfo[i].reg == S390_LAST_ARG_REG)
                                res = FALSE;
                        else {
                                switch(ainfo[i].size) {