commit: 7b28599cfed98fc831c16f1b528f15fd99011dae Author: Sam James <sam <AT> gentoo <DOT> org> AuthorDate: Sun Aug 13 00:20:39 2023 +0000 Commit: Sam James <sam <AT> gentoo <DOT> org> CommitDate: Sun Aug 13 00:20:39 2023 +0000 URL: https://gitweb.gentoo.org/proj/gcc-patches.git/commit/?id=7b28599c
13.2.0: add patch for Botan miscompilation Bug: https://github.com/randombit/botan/issues/3637 Bug: https://gcc.gnu.org/PR110792 Signed-off-by: Sam James <sam <AT> gentoo.org> ...110792-Early-clobber-issues-with-rot32di2.patch | 186 +++++++++++++++++++++ 13.2.0/gentoo/README.history | 3 + 2 files changed, 189 insertions(+) diff --git a/13.2.0/gentoo/84_all_x86_PR110792-Early-clobber-issues-with-rot32di2.patch b/13.2.0/gentoo/84_all_x86_PR110792-Early-clobber-issues-with-rot32di2.patch new file mode 100644 index 0000000..e3c09cc --- /dev/null +++ b/13.2.0/gentoo/84_all_x86_PR110792-Early-clobber-issues-with-rot32di2.patch @@ -0,0 +1,186 @@ +https://gcc.gnu.org/PR110792 +https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=790c1f60a5662b16eb19eb4b81922995863c7571 +https://github.com/randombit/botan/issues/3637 + +From 85628c5653ff40963158a24c60eeec6a3b5a8e56 Mon Sep 17 00:00:00 2001 +From: Roger Sayle <[email protected]> +Date: Thu, 3 Aug 2023 07:12:04 +0100 +Subject: [PATCH] PR target/110792: Early clobber issues with + rot32di2_doubleword on i386. + +This patch is a conservative fix for PR target/110792, a wrong-code +regression affecting doubleword rotations by BITS_PER_WORD, which +effectively swaps the highpart and lowpart words, when the source to be +rotated resides in memory. The issue is that if the register used to +hold the lowpart of the destination is mentioned in the address of +the memory operand, the current define_insn_and_split unintentionally +clobbers it before reading the highpart. + +Hence, for the testcase, the incorrectly generated code looks like: + + salq $4, %rdi // calculate address + movq WHIRL_S+8(%rdi), %rdi // accidentally clobber addr + movq WHIRL_S(%rdi), %rbp // load (wrong) lowpart + +Traditionally, the textbook way to fix this would be to add an +explicit early clobber to the instruction's constraints. + + (define_insn_and_split "<insn>32di2_doubleword" +- [(set (match_operand:DI 0 "register_operand" "=r,r,r") ++ [(set (match_operand:DI 0 "register_operand" "=r,r,&r") + (any_rotate:DI (match_operand:DI 1 "nonimmediate_operand" "0,r,o") + (const_int 32)))] + +but unfortunately this currently generates significantly worse code, +due to a strange choice of reloads (effectively memcpy), which ends up +looking like: + + salq $4, %rdi // calculate address + movdqa WHIRL_S(%rdi), %xmm0 // load the double word in SSE reg. + movaps %xmm0, -16(%rsp) // store the SSE reg back to the stack + movq -8(%rsp), %rdi // load highpart + movq -16(%rsp), %rbp // load lowpart + +Note that reload's "&" doesn't distinguish between the memory being +early clobbered, vs the registers used in an addressing mode being +early clobbered. + +The fix proposed in this patch is to remove the third alternative, that +allowed offsetable memory as an operand, forcing reload to place the +operand into a register before the rotation. This results in: + + salq $4, %rdi + movq WHIRL_S(%rdi), %rax + movq WHIRL_S+8(%rdi), %rdi + movq %rax, %rbp + +I believe there's a more advanced solution, by swapping the order of +the loads (if first destination register is mentioned in the address), +or inserting a lea insn (if both destination registers are mentioned +in the address), but this fix is a minimal "safe" solution, that +should hopefully be suitable for backporting. + +2023-08-03 Roger Sayle <[email protected]> + +gcc/ChangeLog + PR target/110792 + * config/i386/i386.md (<any_rotate>ti3): For rotations by 64 bits + place operand in a register before gen_<insn>64ti2_doubleword. + (<any_rotate>di3): Likewise, for rotations by 32 bits, place + operand in a register before gen_<insn>32di2_doubleword. + (<any_rotate>32di2_doubleword): Constrain operand to be in register. + (<any_rotate>64ti2_doubleword): Likewise. + +gcc/testsuite/ChangeLog + PR target/110792 + * g++.target/i386/pr110792.C: New 32-bit C++ test case. + * gcc.target/i386/pr110792.c: New 64-bit C test case. + +(cherry picked from commit 790c1f60a5662b16eb19eb4b81922995863c7571) +--- + gcc/config/i386/i386.md | 18 ++++++++++++------ + gcc/testsuite/g++.target/i386/pr110792.C | 16 ++++++++++++++++ + gcc/testsuite/gcc.target/i386/pr110792.c | 18 ++++++++++++++++++ + 3 files changed, 46 insertions(+), 6 deletions(-) + create mode 100644 gcc/testsuite/g++.target/i386/pr110792.C + create mode 100644 gcc/testsuite/gcc.target/i386/pr110792.c + +diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md +index f3a3305..a71e837 100644 +--- a/gcc/config/i386/i386.md ++++ b/gcc/config/i386/i386.md +@@ -14359,7 +14359,10 @@ + emit_insn (gen_ix86_<insn>ti3_doubleword + (operands[0], operands[1], operands[2])); + else if (CONST_INT_P (operands[2]) && INTVAL (operands[2]) == 64) +- emit_insn (gen_<insn>64ti2_doubleword (operands[0], operands[1])); ++ { ++ operands[1] = force_reg (TImode, operands[1]); ++ emit_insn (gen_<insn>64ti2_doubleword (operands[0], operands[1])); ++ } + else + { + rtx amount = force_reg (QImode, operands[2]); +@@ -14394,7 +14397,10 @@ + emit_insn (gen_ix86_<insn>di3_doubleword + (operands[0], operands[1], operands[2])); + else if (CONST_INT_P (operands[2]) && INTVAL (operands[2]) == 32) +- emit_insn (gen_<insn>32di2_doubleword (operands[0], operands[1])); ++ { ++ operands[1] = force_reg (DImode, operands[1]); ++ emit_insn (gen_<insn>32di2_doubleword (operands[0], operands[1])); ++ } + else + FAIL; + +@@ -14562,8 +14568,8 @@ + }) + + (define_insn_and_split "<insn>32di2_doubleword" +- [(set (match_operand:DI 0 "register_operand" "=r,r,r") +- (any_rotate:DI (match_operand:DI 1 "nonimmediate_operand" "0,r,o") ++ [(set (match_operand:DI 0 "register_operand" "=r,r") ++ (any_rotate:DI (match_operand:DI 1 "register_operand" "0,r") + (const_int 32)))] + "!TARGET_64BIT" + "#" +@@ -14580,8 +14586,8 @@ + }) + + (define_insn_and_split "<insn>64ti2_doubleword" +- [(set (match_operand:TI 0 "register_operand" "=r,r,r") +- (any_rotate:TI (match_operand:TI 1 "nonimmediate_operand" "0,r,o") ++ [(set (match_operand:TI 0 "register_operand" "=r,r") ++ (any_rotate:TI (match_operand:TI 1 "register_operand" "0,r") + (const_int 64)))] + "TARGET_64BIT" + "#" +diff --git a/gcc/testsuite/g++.target/i386/pr110792.C b/gcc/testsuite/g++.target/i386/pr110792.C +new file mode 100644 +index 0000000..ce21a7a +--- /dev/null ++++ b/gcc/testsuite/g++.target/i386/pr110792.C +@@ -0,0 +1,16 @@ ++/* { dg-do compile { target ia32 } } */ ++/* { dg-options "-O2" } */ ++ ++template <int ROT, typename T> ++inline T rotr(T input) ++{ ++ return static_cast<T>((input >> ROT) | (input << (8 * sizeof(T) - ROT))); ++} ++ ++unsigned long long WHIRL_S[256] = {0x18186018C07830D8}; ++unsigned long long whirl(unsigned char x0) ++{ ++ const unsigned long long s4 = WHIRL_S[x0&0xFF]; ++ return rotr<32>(s4); ++} ++/* { dg-final { scan-assembler-not "movl\tWHIRL_S\\+4\\(,%eax,8\\), %eax" } } */ +diff --git a/gcc/testsuite/gcc.target/i386/pr110792.c b/gcc/testsuite/gcc.target/i386/pr110792.c +new file mode 100644 +index 0000000..b65125c +--- /dev/null ++++ b/gcc/testsuite/gcc.target/i386/pr110792.c +@@ -0,0 +1,18 @@ ++/* { dg-do compile { target int128 } } */ ++/* { dg-options "-O2" } */ ++ ++static inline unsigned __int128 rotr(unsigned __int128 input) ++{ ++ return ((input >> 64) | (input << (64))); ++} ++ ++unsigned __int128 WHIRL_S[256] = {((__int128)0x18186018C07830D8) << 64 |0x18186018C07830D8}; ++unsigned __int128 whirl(unsigned char x0) ++{ ++ register int t __asm("rdi") = x0&0xFF; ++ const unsigned __int128 s4 = WHIRL_S[t]; ++ register unsigned __int128 tt __asm("rdi") = rotr(s4); ++ asm("":::"memory"); ++ return tt; ++} ++/* { dg-final { scan-assembler-not "movq\tWHIRL_S\\+8\\(%rdi\\), %rdi" } } */ +-- +2.41.0 + diff --git a/13.2.0/gentoo/README.history b/13.2.0/gentoo/README.history index 769413a..2f1fc73 100644 --- a/13.2.0/gentoo/README.history +++ b/13.2.0/gentoo/README.history @@ -1,3 +1,6 @@ +6 13 Aug 2023 + + 84_all_x86_PR110792-Early-clobber-issues-with-rot32di2.patch + 5 05 Aug 2023 - 82_all_arm64_PR110280_ICE_fold-const.patch
