[pushed,PR122215,IRA] : Fix undefined behaviour of improve_allocation

Message ID 8c9e6908-4e3d-48b3-87d0-b31ede4dc818@redhat.com
State Committed
Headers
Series [pushed,PR122215,IRA] : Fix undefined behaviour of improve_allocation |

Checks

Context Check Description
rivoscibot/toolchain-ci-rivos-lint success Lint passed
rivoscibot/toolchain-ci-rivos-apply-patch success Patch applied
rivoscibot/toolchain-ci-rivos-build--newlib-rv64gcv-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-build--linux-rv64gcv-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-build--linux-rv64gc_zba_zbb_zbc_zbs-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-test success Testing passed

Commit Message

Vladimir Makarov Dec. 5, 2025, 7:30 p.m. UTC
  The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122215

The patch was successfully bootstrapped and tested on x86-64.
  

Comments

Peter Bergner Dec. 10, 2025, 4:31 p.m. UTC | #1
On 12/5/25 11:30 AM, Vladimir Makarov wrote:
> The following patch fixes
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122215

I assume this is something we'll want back ported after some burn-in
time on trunk?

Peter
  
Jeffrey Law Dec. 11, 2025, 2:49 a.m. UTC | #2
Yea, it's on the weekly tracker, so barring surprises I'll probably
backport it Monday in preparation for the Tuesday meeting.

On Wed, Dec 10, 2025 at 8:31 AM Peter Bergner <bergner@tenstorrent.com>
wrote:

> On 12/5/25 11:30 AM, Vladimir Makarov wrote:
> > The following patch fixes
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122215
>
> I assume this is something we'll want back ported after some burn-in
> time on trunk?
>
> Peter
>
>
>
  
Vladimir Makarov Dec. 11, 2025, 3:19 p.m. UTC | #3
On 12/10/25 9:49 PM, Jeff Law wrote:
> Yea, it's on the weekly tracker, so barring surprises I'll probably 
> backport it Monday in preparation for the Tuesday meeting.
>
I've just backported the patch to affected release branches (gcc-14 and 
gcc-15).  The patch for gcc-14 does not have the testcase and changes in 
dejagnu as it requires more work to include them, e.g. adding 
sparseset.supp file absent in gcc-14 branch.


> On Wed, Dec 10, 2025 at 8:31 AM Peter Bergner 
> <bergner@tenstorrent.com> wrote:
>
>     On 12/5/25 11:30 AM, Vladimir Makarov wrote:
>     > The following patch fixes
>     >
>     > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122215
>
>     I assume this is something we'll want back ported after some burn-in
>     time on trunk?
>
  

Patch

commit 6ca1fb648daecaa9c843ff05af304c1fc1a8fcb5
Author: Vladimir N. Makarov <vmakarov@redhat.com>
Date:   Fri Dec 5 14:21:38 2025 -0500

    [PR122215, IRA]: Fix undefined behaviour of improve_allocation
    
    Register filters are used in one loop of improve_allocation to ignore some
    hard regs for cost calculation but it is missed in the subsequent loop
    using the costs.  This results in usage of random (undefined) register costs
    and in sporadic code generation for riscv32 which uses the filters.
    
    gcc/ChangeLog:
    
            PR rtl-optimization/122215
            * ira-color.cc (improve_allocation): Use register filter for all
            loop on hard regs.
    
    gcc/testsuite/ChangeLog:
    
            PR rtl-optimization/122215
            * gcc.target/riscv/pr122215.c: New.
            * lib/target-supports.exp (check_effective_target_valgrind): New.

diff --git a/gcc/ira-color.cc b/gcc/ira-color.cc
index 4ee2a65e291..a051db27fbf 100644
--- a/gcc/ira-color.cc
+++ b/gcc/ira-color.cc
@@ -3369,6 +3369,9 @@  improve_allocation (void)
       for (j = 0; j < class_size; j++)
 	{
 	  hregno = ira_class_hard_regs[aclass][j];
+	  if (NUM_REGISTER_FILTERS
+	      && !test_register_filters (ALLOCNO_REGISTER_FILTERS (a), hregno))
+	    continue;
 	  if (check_hard_reg_p (a, hregno,
 				conflicting_regs, profitable_hard_regs)
 	      && min_cost > costs[hregno])
diff --git a/gcc/testsuite/gcc.target/riscv/pr122215.c b/gcc/testsuite/gcc.target/riscv/pr122215.c
new file mode 100644
index 00000000000..cdc1ed7c4e7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/pr122215.c
@@ -0,0 +1,46 @@ 
+/* { dg-do compile } */
+/* { dg-require-effective-target valgrind } */
+/* { dg-additional-files "sparseset.supp" } */
+/* { dg-options "-wrapper valgrind,-q,--exit-on-first-error=yes,--error-exitcode=1,--suppressions=${srcdir}/sparseset.supp" } */
+
+typedef signed int int32_t;
+typedef signed long int int64_t;
+
+int64_t dual_reg_insn(int64_t x) {
+    int64_t res;
+    int64_t zero = 0;
+    asm ("some_custom_insn %0,%1,%2" : "=R" (res) : "R" (x), "R" (zero));
+    return res;
+}
+
+int32_t single_reg_insn(int32_t x) {
+    int32_t res;
+    int32_t zero = 0;
+    asm ("some_custom_insn %0,%1,%2" : "=r" (res) : "r" (x), "r" (zero));
+    return res;
+}
+
+int32_t single_reg_insn_explicit_zero(int32_t x) {
+    int32_t res;
+    asm ("some_custom_insn %0,%1,%2" : "=r" (res) : "r" (x), "r" (0));
+    return res;
+}
+
+int64_t dual_reg_insn2(int64_t x) {
+    int64_t res;
+    int64_t zero = 0;
+    asm ("some_custom_insn %0,%1,%2" : "=R" (res) : "R" (x), "R" (zero));
+    return res;
+    /* This function is IDENTICAL to dual_reg_insn,
+     * but for some obscure reason (alignment?)
+     * it decides to use sX registers instead of aX to store zero,
+     * resulting in a much larger code since it needs to use the stack.
+     * THIS ONLY HAPPENS SOMETIMES!
+     */
+}
+
+int64_t dual_reg_insn_explicit_zero(int64_t x) {
+    int64_t res;
+    asm ("some_custom_insn %0,%1,%2" : "=R" (res) : "R" (x), "R" (0LL));
+    return res;
+}
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 1df80d412da..6251f4e58f8 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -14762,3 +14762,11 @@  proc check_effective_target_fentry { } {
 	}
     }]
 }
+
+# Check if valgrind executable exists in PATH on host
+proc check_effective_target_valgrind { } {
+    if { [which valgrind] != 0 } {
+        return 1
+    }
+    return 0
+}