[llvm] 43a0be0 - [X86] Record the enclosed register in X86DomainReassignment::buildClosure (#202534)

[via llvm-commits]

Author: mbhade-amd
Date: 2026-06-15T11:42:40+05:30
New Revision: 43a0be066b5d0f897ae6191820aec4ecd821b32e

URL: https://github.com/llvm/llvm-project/commit/43a0be066b5d0f897ae6191820aec4ecd821b32e
DIFF: https://github.com/llvm/llvm-project/commit/43a0be066b5d0f897ae6191820aec4ecd821b32e.diff

LOG: [X86] Record the enclosed register in X86DomainReassignment::buildClosure (#202534)

buildClosure recorded the seed register Reg in the function-wide
EnclosedEdges map on every worklist iteration instead of CurReg, the
register actually being added to the closure. EnclosedEdges therefore
only ever contained the seed of each closure.

The driver loop in runOnMachineFunction skips registers already present
in EnclosedEdges before starting a new closure. Because only seeds were
recorded, every non-seed member of an already-built closure looked like
a fresh seed, so a redundant closure was built for it and then
immediately discarded by the EnclosedInstrs cross-closure check. The
emitted code is unchanged; the pass just performed redundant work
proportional to closure size.

Key EnclosedEdges by CurReg so each enclosed register is recorded once.

This was found as part of @jlebar's X86 LLVM bug hunt / FuzzX effort:

https://github.com/SemiAnalysisAI/FuzzX/tree/master/x86/bugs/007-domain-reassignment-wrong-enclosed-key

cc @jlebar

Added: 
    llvm/test/CodeGen/X86/domain-reassignment-closure-stats.mir

Modified: 
    llvm/lib/Target/X86/X86DomainReassignment.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/X86/X86DomainReassignment.cpp b/llvm/lib/Target/X86/X86DomainReassignment.cpp
index a9f68393aa3e2..8afa1cbc78e03 100644
--- a/llvm/lib/Target/X86/X86DomainReassignment.cpp
+++ b/llvm/lib/Target/X86/X86DomainReassignment.cpp
@@ -33,6 +33,7 @@ using namespace llvm;
 #define DEBUG_TYPE "x86-domain-reassignment"
 
 STATISTIC(NumClosuresConverted, "Number of closures converted by the pass");
+STATISTIC(NumClosuresBuilt, "Number of closures built by the pass");
 
 static cl::opt<bool> DisableX86DomainReassignment(
     "disable-x86-domain-reassignment", cl::Hidden,
@@ -553,7 +554,7 @@ void X86DomainReassignmentImpl::buildClosure(Closure &C, Register Reg) {
     // Register already in this closure.
     if (!C.insertEdge(CurReg))
       continue;
-    EnclosedEdges[Reg] = C.getID();
+    EnclosedEdges[CurReg] = C.getID();
 
     MachineInstr *DefMI = MRI->getVRegDef(CurReg);
     if (!encloseInstr(C, DefMI))
@@ -812,6 +813,7 @@ bool X86DomainReassignmentImpl::runOnMachineFunction(MachineFunction &MF) {
     // Calculate closure starting with Reg.
     Closure C(ClosureID++, {MaskDomain});
     buildClosure(C, Reg);
+    ++NumClosuresBuilt;
 
     // Collect all closures that can potentially be converted.
     if (!C.empty() && C.isLegal(MaskDomain))

diff  --git a/llvm/test/CodeGen/X86/domain-reassignment-closure-stats.mir b/llvm/test/CodeGen/X86/domain-reassignment-closure-stats.mir
new file mode 100644
index 0000000000000..85c401ce1f807
--- /dev/null
+++ b/llvm/test/CodeGen/X86/domain-reassignment-closure-stats.mir
@@ -0,0 +1,92 @@
+# REQUIRES: asserts
+# RUN: llc -run-pass x86-domain-reassignment -mtriple=x86_64-unknown-unknown \
+# RUN:   -mattr=+avx512f,+avx512bw,+avx512dq -stats %s -o /dev/null 2>&1 \
+# RUN:   | FileCheck %s
+
+# Regression test for buildClosure recording the seed register instead of the
+# register actually added to the closure. Before the fix EnclosedEdges only ever
+# held the seed of each closure, so every other member of an already-built
+# closure looked like a fresh seed and a redundant closure was built for it and
+# immediately discarded. The generated code is identical either way (the
+# converted-closure count is unaffected); only the amount of redundant work
+# changes. For test_8bitops the closure has eight members: buggy builds 12
+# closures, the fix builds 2. The number of converted closures stays at 1.
+
+# The leading {{^ *}} anchor prevents a stale "12" count from matching "2".
+# CHECK-DAG: {{^ *}}2 x86-domain-reassignment - Number of closures built by the pass
+# CHECK-DAG: {{^ *}}1 x86-domain-reassignment - Number of closures converted by the pass
+
+--- |
+  target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+  target triple = "x86_64-unknown-unknown"
+
+  define void @test_8bitops() #0 {
+    ret void
+  }
+
+  attributes #0 = { "target-cpu"="skylake-avx512" }
+...
+---
+name:            test_8bitops
+alignment:       16
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: gr64, preferred-register: '' }
+  - { id: 1, class: vr512, preferred-register: '' }
+  - { id: 2, class: vr512, preferred-register: '' }
+  - { id: 3, class: vr512, preferred-register: '' }
+  - { id: 4, class: vr512, preferred-register: '' }
+  - { id: 5, class: vk8, preferred-register: '' }
+  - { id: 6, class: gr32, preferred-register: '' }
+  - { id: 7, class: gr8, preferred-register: '' }
+  - { id: 8, class: gr32, preferred-register: '' }
+  - { id: 9, class: gr32, preferred-register: '' }
+  - { id: 10, class: vk8wm, preferred-register: '' }
+  - { id: 11, class: vr512, preferred-register: '' }
+  - { id: 12, class: gr8, preferred-register: '' }
+  - { id: 13, class: gr8, preferred-register: '' }
+  - { id: 14, class: gr8, preferred-register: '' }
+  - { id: 15, class: gr8, preferred-register: '' }
+  - { id: 16, class: gr8, preferred-register: '' }
+  - { id: 17, class: gr8, preferred-register: '' }
+  - { id: 18, class: gr8, preferred-register: '' }
+liveins:
+  - { reg: '$rdi', virtual-reg: '%0' }
+  - { reg: '$zmm0', virtual-reg: '%1' }
+  - { reg: '$zmm1', virtual-reg: '%2' }
+  - { reg: '$zmm2', virtual-reg: '%3' }
+  - { reg: '$zmm3', virtual-reg: '%4' }
+body:             |
+  bb.0:
+    liveins: $rdi, $zmm0, $zmm1, $zmm2, $zmm3
+
+    %0 = COPY $rdi
+    %1 = COPY $zmm0
+    %2 = COPY $zmm1
+    %3 = COPY $zmm2
+    %4 = COPY $zmm3
+
+    %5 = VCMPPDZrri %3, %4, 0, implicit $mxcsr
+    %6 = COPY %5
+    %7 = COPY %6.sub_8bit
+
+    %12 = SHR8ri %7, 2, implicit-def dead $eflags
+    %13 = SHL8ri %12, 1, implicit-def dead $eflags
+    %14 = NOT8r %13
+    %15 = OR8rr %14, %12, implicit-def dead $eflags
+    %16 = AND8rr %15, %13, implicit-def dead $eflags
+    %17 = XOR8rr %16, %12, implicit-def dead $eflags
+    %18 = ADD8rr %17, %14, implicit-def dead $eflags
+
+    %8 = IMPLICIT_DEF
+    %9 = INSERT_SUBREG %8, %18, %subreg.sub_8bit_hi
+    %10 = COPY %9
+    %11 = VMOVAPDZrrk %2, killed %10, %1
+    VMOVAPDZmr %0, 1, $noreg, 0, $noreg, killed %11
+
+  bb.1:
+
+  bb.2:
+    RET 0
+
+...