On Sat, Aug 12, 2023 at 09:04:19AM +0800, Xi Ruoyao wrote:
> On Thu, 2023-08-10 at 15:04 +0200, Stefan Schulze Frielinghaus via Gcc-
> patches wrote:
> > In the former fix in commit 41ef5a34161356817807be3a2e51fbdbe575ae85 I
> > completely missed the fact that the normal form of a generated constant for
> > a
> > mode with fewer bits than in HOST_WIDE_INT is a sign extended version of the
> > actual constant. This even holds true for unsigned constants.
> >
> > Fixed by masking out the upper bits for the incoming constant and sign
> > extending the resulting unsigned constant.
> >
> > Bootstrapped and regtested on x64 and s390x. Ok for mainline?
>
> The patch fails to apply:
>
> patching file gcc/combine.cc
> Hunk #1 FAILED at 11923.
> Hunk #2 FAILED at 11962.
>
> It looks like some indents are tabs in the source file, but white spaces
> in the patch.
The patch itself applies cleanly. This is due to my inlined diff in
order to raise some discussion, i.e., just remove the following from
the email and the patch applies:
> > diff --git a/gcc/combine.cc b/gcc/combine.cc
> > index 468b7fde911..80c4ff0fbaf 100644
> > --- a/gcc/combine.cc
> > +++ b/gcc/combine.cc
> > @@ -11923,7 +11923,7 @@ simplify_compare_const (enum rtx_code code,
> > machine_mode mode,
> > /* (unsigned) < 0x80000000 is equivalent to >= 0. */
> > else if (is_a <scalar_int_mode> (mode, &int_mode)
> > && GET_MODE_PRECISION (int_mode) - 1 < HOST_BITS_PER_WIDE_INT
> > - && ((unsigned HOST_WIDE_INT) const_op
> > + && (((unsigned HOST_WIDE_INT) const_op & GET_MODE_MASK
> > (int_mode))
> > == HOST_WIDE_INT_1U << (GET_MODE_PRECISION (int_mode) -
> > 1)))
> > {
> > const_op = 0;
> > @@ -11962,7 +11962,7 @@ simplify_compare_const (enum rtx_code code,
> > machine_mode mode,
> > /* (unsigned) >= 0x80000000 is equivalent to < 0. */
> > else if (is_a <scalar_int_mode> (mode, &int_mode)
> > && GET_MODE_PRECISION (int_mode) - 1 < HOST_BITS_PER_WIDE_INT
> > - && ((unsigned HOST_WIDE_INT) const_op
> > + && (((unsigned HOST_WIDE_INT) const_op & GET_MODE_MASK
> > (int_mode))
> > == HOST_WIDE_INT_1U << (GET_MODE_PRECISION (int_mode) -
> > 1)))
> > {
> > const_op = 0;
Looks like git am/apply is confused by that.
Cheers,
Stefan
> >
> > For example, while bootstrapping on x64 the optimization is missed since
> > a LTU comparison in QImode is done and the constant equals
> > 0xffffffffffffff80.
> >
> > Sorry for inlining another patch, but I would really like to make sure
> > that my understanding is correct, now, before I come up with another
> > patch. Thus it would be great if someone could shed some light on this.
> >
> > gcc/ChangeLog:
> >
> > * combine.cc (simplify_compare_const): Properly handle unsigned
> > constants while narrowing comparison of memory and constants.
> > ---
> > gcc/combine.cc | 19 ++++++++++---------
> > 1 file changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/gcc/combine.cc b/gcc/combine.cc
> > index e46d202d0a7..468b7fde911 100644
> > --- a/gcc/combine.cc
> > +++ b/gcc/combine.cc
> > @@ -12003,14 +12003,15 @@ simplify_compare_const (enum rtx_code code,
> > machine_mode mode,
> > && !MEM_VOLATILE_P (op0)
> > /* The optimization makes only sense for constants which are big
> > enough
> > so that we have a chance to chop off something at all. */
> > - && (unsigned HOST_WIDE_INT) const_op > 0xff
> > - /* Bail out, if the constant does not fit into INT_MODE. */
> > - && (unsigned HOST_WIDE_INT) const_op
> > - < ((HOST_WIDE_INT_1U << (GET_MODE_PRECISION (int_mode) - 1) << 1)
> > - 1)
> > + && ((unsigned HOST_WIDE_INT) const_op & GET_MODE_MASK (int_mode)) >
> > 0xff
> > /* Ensure that we do not overflow during normalization. */
> > - && (code != GTU || (unsigned HOST_WIDE_INT) const_op <
> > HOST_WIDE_INT_M1U))
> > + && (code != GTU
> > + || ((unsigned HOST_WIDE_INT) const_op & GET_MODE_MASK (int_mode))
> > + < HOST_WIDE_INT_M1U)
> > + && trunc_int_for_mode (const_op, int_mode) == const_op)
> > {
> > - unsigned HOST_WIDE_INT n = (unsigned HOST_WIDE_INT) const_op;
> > + unsigned HOST_WIDE_INT n
> > + = (unsigned HOST_WIDE_INT) const_op & GET_MODE_MASK (int_mode);
> > enum rtx_code adjusted_code;
> >
> > /* Normalize code to either LEU or GEU. */
> > @@ -12051,15 +12052,15 @@ simplify_compare_const (enum rtx_code code,
> > machine_mode mode,
> > HOST_WIDE_INT_PRINT_HEX ") to (MEM %s "
> > HOST_WIDE_INT_PRINT_HEX ").\n", GET_MODE_NAME (int_mode),
> > GET_MODE_NAME (narrow_mode_iter), GET_RTX_NAME (code),
> > - (unsigned HOST_WIDE_INT)const_op, GET_RTX_NAME
> > (adjusted_code),
> > - n);
> > + (unsigned HOST_WIDE_INT) const_op & GET_MODE_MASK
> > (int_mode),
> > + GET_RTX_NAME (adjusted_code), n);
> > }
> > poly_int64 offset = (BYTES_BIG_ENDIAN
> > ? 0
> > : (GET_MODE_SIZE (int_mode)
> > - GET_MODE_SIZE (narrow_mode_iter)));
> > *pop0 = adjust_address_nv (op0, narrow_mode_iter, offset);
> > - *pop1 = GEN_INT (n);
> > + *pop1 = gen_int_mode (n, narrow_mode_iter);
> > return adjusted_code;
> > }
> > }
>
> --
> Xi Ruoyao <xry...@xry111.site>
> School of Aerospace Science and Technology, Xidian University
