On Tue, Aug 05, 2025 at 04:55:37PM +0200, Stefan Schulze Frielinghaus wrote:
> On Tue, Aug 05, 2025 at 04:25:43PM +0200, Jakub Jelinek wrote:
> > On Tue, Aug 05, 2025 at 04:23:14PM +0200, Stefan Schulze Frielinghaus wrote:
> > > From: Stefan Schulze Frielinghaus <stefa...@linux.ibm.com>
> > >
> > > From: Stefan Schulze Frielinghaus <stefa...@gcc.gnu.org>
> > >
> > > gcc/ChangeLog:
> > >
> > > * explow.cc (promote_function_mode): Allow targets to promote
> > > _BitInt arguments.
> > > ---
> > > Bootstrapped and regtested on s390. Ok for mainline?
> >
> > Do you need this with r16-2778-ga9b96c63d9f1c5cb4a6 now being committed?
>
> Oh I haven't seen the commit from today. LGTM and we can probably drop
> this patch. I will do another round of bootstrap+regtest and will
> commit the other two patches eventually.
After rebasing I see gcc.dg/torture/bitint-64.c failing after
r16-2779-geed0f5fa0e1e29 but I think this is not the real culprit, i.e.,
even with r16-2778-ga9b96c63d9f1c5cb4a6 we return non-sign-extended
value 22 from foo() but "unnecessarily" sign extend it after the call
which is why we don't abort in this case. With r16-2779-geed0f5fa0e1e29
the "unnecessary" sign extend after the call is removed and we abort.
Long story short, there is a sign extend missing before returning from
foo(). The return block
<bb 5> [local count: 1073312328]:
_10 = __atomic_load_1 (&b, 5);
_11 = VIEW_CONVERT_EXPR<_BitInt(5)>(_10);
b ={v} {CLOBBER(eos)};
return _11;
is expanded into
(note 75 74 76 12 [bb 12] NOTE_INSN_BASIC_BLOCK)
(insn 76 75 77 12 (parallel [
(asm_operands/v ("") ("") 0 []
[]
[])
(clobber (mem:BLK (scratch) [0 A8]))
]) "src/gcc/testsuite/gcc.dg/torture/bitint-64.c":14:10 -1
(nil))
(insn 77 76 78 12 (set (reg:DI 107)
(zero_extend:DI (mem/v:QI (plus:DI (reg/f:DI 55 virtual-stack-vars)
(const_int -2 [0xfffffffffffffffe])) [-1 S1 A16])))
"src/gcc/testsuite/gcc.dg/torture/bitint-64.c":14:10 -1
(nil))
(insn 78 77 79 12 (set (reg:QI 68 [ _10 ])
(subreg:QI (reg:DI 107) 7))
"src/gcc/testsuite/gcc.dg/torture/bitint-64.c":14:10 -1
(nil))
(insn 79 78 80 12 (parallel [
(asm_operands/v ("") ("") 0 []
[]
[])
(clobber (mem:BLK (scratch) [0 A8]))
]) "src/gcc/testsuite/gcc.dg/torture/bitint-64.c":14:10 -1
(nil))
(insn 80 79 81 12 (set (reg:DI 109)
(sign_extend:DI (reg:QI 68 [ _10 ])))
"src/gcc/testsuite/gcc.dg/torture/bitint-64.c":14:10 discrim 2 -1
(nil))
(insn 81 80 85 12 (set (reg:DI 70 [ <retval>+-7 ])
(reg:DI 109)) "src/gcc/testsuite/gcc.dg/torture/bitint-64.c":14:10
discrim 2 -1
(nil))
(insn 85 81 86 12 (set (reg/i:DI 2 %r2)
(reg:DI 70 [ <retval>+-7 ]))
"src/gcc/testsuite/gcc.dg/torture/bitint-64.c":15:1 -1
(nil))
(insn 86 85 0 12 (use (reg/i:DI 2 %r2))
"src/gcc/testsuite/gcc.dg/torture/bitint-64.c":15:1 -1
(nil))
At first it looks like as if insn 80 is supposed to sign extend the
return value which wouldn't work since a _BitInt(5) is sign extended via
left and right shifts. However, since the value is coming from memory
I'm not sure if that value is already supposed to be sign extended which
then would be ok again. If not a _BitInt(5) cast after the
VIEW_CONVERT_EXPR might solve this. I will have a second look at it
tomorrow.
Cheers,
Stefan
