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. -- Xi Ruoyao <xry...@xry111.site>
