在 2025/7/4 上午11:14, Xi Ruoyao 写道:
On Fri, 2025-07-04 at 09:47 +0800, Lulu Cheng wrote:
在 2025/7/2 下午3:31, Xi Ruoyao 写道:
The register_operand predicate can match subreg, then we'd have a subreg
of subreg and it's invalid.  Use lowpart_subreg to avoid the nested
   subreg.

gcc/ChangeLog:

        * config/loongarch/loongarch.md (crc_combine): Avoid nested
        subreg.

gcc/testsuite/ChangeLog:

        * gcc.c-torture/compile/pr120708.c: New test.
---

Bootstrapped and regtested on loongarch64-linux-gnu.  Ok for trunk and
releases/gcc-15?
Hi,

I'm OK with this patch.
Will add PR120728 to the subject and ChangeLog then push.

But I have doubts about the following template:
See below (I decided not to remove the replied text to allow people
reading the mail easier).

```

(define_insn_and_split "*crc_combine"
    [(set (match_operand:SI 0 "register_operand" "=r,r")
          (unspec:SI
            [(reg:SUBDI 0)
             (subreg:SI
               (xor:DI
                 (match_operand:DI 1 "register_operand" "r,r")
                 ; Our LOAD_EXTEND_OP makes this same as sign_extend
                 ; if SUBDI is SI, or zero_extend if SUBDI is QI or HI.
                 ; For the former the high bits in rk are ignored by
                 ; crc.w.w.w anyway, for the latter the zero extension is
                 ; necessary for the correctness of this transformation.
                 (subreg:DI
                   (match_operand:SUBDI 2 "memory_operand" "m,k") 0)) 0)]
            CRC))]

    "TARGET_64BIT && loongarch_pre_reload_split ()"

```

The use of subreg in gccint is described as follows, I don't understand,
is subreg mem available before sched, or is it just not recommended to
use it?

```

mem subregs of mem were common in earlier versions of GCC and are still
supported. During the reload pass these are replaced by plain mems. On
machines that do not do instruction scheduling, use of subregs of mem
are still used, but this is no longer recommended. Such subregs are
considered to be register_operands rather than memory_operands before
and during reload. Because of this, the scheduling passes cannot
properly schedule instructions with subregs of mem, so for machines that
do scheduling, subregs of mem should never be used. To support this, the
combine and recog passes have explicit code to inhibit the creation of
subregs of mem when INSN_SCHEDULING is defined.

The use of subregs of mem after the reload pass is an area that is not
well understood and should be avoided. There is still some code in the
compiler to support this, but this code has possibly rotted. This use of
subregs is discouraged and will most likely not be supported in the future.
I suppose it should be fine as long as we always rewrite the instruction
(at split1 in this case) to remove the (subreg (mem)), similar to the
pre_reload_split hack (that gccint doesn't document either) in i386,
loongarch and some other backends.

In s390 backend the llgt_sidi pattern also matches a paradoxical (subreg
(mem)) and split it immediately on split1.

And I guess the doc is a bit rotten here.  i386 has already encountered
some ICE in the recent years because combine produces a (subreg (mem))
and the fix was just allowing (subreg (mem)) in the backend, see
https://gcc.gnu.org/PR105927 for example.

s390 and i386 definitely have sched.

I agree with you.

Thanks!

Reply via email to