在 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.
But I have doubts about the following template:
```
(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.
```
gcc/config/loongarch/loongarch.md | 3 ++-
.../gcc.c-torture/compile/pr120708.c | 20 +++++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr120708.c
diff --git a/gcc/config/loongarch/loongarch.md
b/gcc/config/loongarch/loongarch.md
index a13398fdff4..8cf2ac90c64 100644
--- a/gcc/config/loongarch/loongarch.md
+++ b/gcc/config/loongarch/loongarch.md
@@ -4603,9 +4603,10 @@ (define_insn_and_split "*crc_combine"
"&& true"
[(set (match_dup 3) (match_dup 2))
(set (match_dup 0)
- (unspec:SI [(match_dup 3) (subreg:SI (match_dup 1) 0)] CRC))]
+ (unspec:SI [(match_dup 3) (match_dup 1)] CRC))]
{
operands[3] = gen_reg_rtx (<MODE>mode);
+ operands[1] = lowpart_subreg (SImode, operands[1], DImode);
})
;; With normal or medium code models, if the only use of a pc-relative
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr120708.c
b/gcc/testsuite/gcc.c-torture/compile/pr120708.c
new file mode 100644
index 00000000000..9b37e608d7f
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr120708.c
@@ -0,0 +1,20 @@
+typedef __UINT8_TYPE__ uint8_t;
+typedef __UINT32_TYPE__ uint32_t;
+
+typedef struct
+{
+ uint32_t dword[2];
+ uint8_t byte[8];
+} reg64_t;
+reg64_t TestF20F_opgd, TestF20F_oped;
+
+void
+TestF20F ()
+{
+ TestF20F_opgd.dword[0] ^= TestF20F_oped.byte[0];
+ for (int i = 0; i < 8; i++)
+ if (TestF20F_opgd.dword[0] & 1)
+ TestF20F_opgd.dword[0] = TestF20F_opgd.dword[0] >> 1 ^
(uint32_t)2197175160UL;
+ else
+ TestF20F_opgd.dword[0] = TestF20F_opgd.dword[0] >> 1;
+}