Re: [PULL 11/24] tcg/optimize: Use tcg_constant_internal with constant folding

[Philippe Mathieu-Daudé]

On 2/4/21 10:37 AM, David Hildenbrand wrote:
> On 04.02.21 10:29, Richard W.M. Jones wrote:
>>>> commit 8f17a975e60b773d7c366a81c0d9bbe304f30859
>>>> Author: Richard Henderson <richard.henderson@linaro.org>
>>>> Date:   Mon Mar 30 19:52:02 2020 -0700
>>>>
>>>>      tcg/optimize: Adjust TempOptInfo allocation
>>>>
>>>> The image boots just fine on s390x/TCG as well.
>>>
>>> Let me try this in a minute on my original test machine.
>>
>> I got the wrong end of the stick as David pointed out in the other email.
>>
>> However I did test things again this morning (all on s390 host), and
>> current head (1ed9228f63ea4b) fails same as before ("mount" command
>> fails).
>>
>> Also I downloaded:
>>
>>   
>> https://dl.fedoraproject.org/pub/fedora-secondary/releases/33/Cloud/s390x/images/Fedora-Cloud-Base-33-1.2.s390x.qcow2
>>
>>
>> and booted it on 1ed9228f63ea4b using this command:
>>
>>    $ ~/d/qemu/build/s390x-softmmu/qemu-system-s390x -machine accel=tcg
>> -m 2048 -drive
>> file=Fedora-Cloud-Base-33-1.2.s390x.qcow2,format=qcow2,if=virtio
>> -serial stdio
>>
>> Lots of core dumps inside the guest, same as David saw.
>>
>> I then reset qemu back to 8f17a975e60b773d ("tcg/optimize: Adjust
>> TempOptInfo allocation"), rebuilt qemu, tested the same command and
>> cloud image, and that booted up much happier with no failures or core
>> dumps.
>>
>> Isn't it kind of weird that this would only affect an s390 host?  I
>> don't understand why the host would make a difference if we're doing
>> TCG.
> 
> I assume an existing BUG in the s390x TCG backend ... which makes it
> harder to debug :)

This seems to fix it:

-- >8 --
diff --git a/tcg/s390/tcg-target.c.inc b/tcg/s390/tcg-target.c.inc
index 8517e552323..32d03dbfbaf 100644
--- a/tcg/s390/tcg-target.c.inc
+++ b/tcg/s390/tcg-target.c.inc
@@ -1094,10 +1094,16 @@ static int tgen_cmp(TCGContext *s, TCGType type,
TCGCond c, TCGReg r1,
                 op = (is_unsigned ? RIL_CLFI : RIL_CFI);
                 tcg_out_insn_RIL(s, op, r1, c2);
                 goto exit;
-            } else if (c2 == (is_unsigned ? (uint32_t)c2 : (int32_t)c2)) {
-                op = (is_unsigned ? RIL_CLGFI : RIL_CGFI);
-                tcg_out_insn_RIL(s, op, r1, c2);
-                goto exit;
+            } else if (is_unsigned) {
+                if (c2 == (uint32_t)c2) {
+                    tcg_out_insn_RIL(s, RIL_CLGFI, r1, c2);
+                    goto exit;
+                }
+            } else {
+                if (c2 == (int32_t)c2) {
+                    tcg_out_insn_RIL(s, RIL_CGFI, r1, c2);
+                    goto exit;
+                }
             }
         }
---