On Wed, May 21, 2025 at 10:13:27AM +0800, Yang Yujie wrote:
> On Tue, May 20, 2025 at 03:44:09PM GMT, Jakub Jelinek wrote:
> > I'd suggest working on it incrementally rather than with a full patch set.
> > In one or multiple patches handle the promote_mode stuff, the atomic
> > extension and expr.cc changes with the feedback incorporated.
>
> Ok.
>
> > For gimple-lower-bitint.cc I'd really like to see what testing you've done
> > to decide on a case by case basis.
> >
> > > > Are you sure all those changes were really necessary (rather than doing
> > > > them
> > > > just in case)? I believe most of gimple-lower-bitint.cc already should
> > > > be
> > > > sign or zero extending the partial limbs when storing stuff, there can
> > > > be
> > > > some corner cases (I think one of the shift directions at least).
> > >
> > > The modifications to gimple-lower-bitint.cc are based on testing,
> >
> > The tests weren't included :(.
>
> "The tests" refer to the existing regression tests in testsuite/gcc.dg and
> testsuite/gcc.dg/torture, specifically bitint-*.c.
The existing testsuite never tests whether the padding bits are sign or zero
extended or contain unknown values.
In the s390x patch, info->extended is set to true, yet all bitint tests but
bitint-64.c at -O3 pass.
So, I'm really interested to know in which cases you found the existing code
not doing the extensions, ideally simple {,un}signed _BitInt(N) with values
X and Y doing arithmetic or logical operation OP.
And put that into the new test(s), so that we can ensure that the extensions
are performed correctly.
Jakub
