Skip to content

Conversation

@RKSimon
Copy link
Collaborator

@RKSimon RKSimon commented Nov 6, 2025

#166337 replaces large (illegal type) loads/stores with a smaller i32 load/store based off the demanded shifted bits. As these shifts are non-constant we can't regenerate the PointerInfo data with a fixed offset, so we need to discard the data entirely.

Fixes #166744 - post-ra has to reconstruct dependencies after the chains have been stripped and uses pointer info instead - which resulted in some loads being rescheduled earlier than the dependent store as it was thought they didn't alias

…unknown offset load/stores

llvm#166337 replaces large (illegal type) loads/stores with a smaller i32 load/store based off the demanded shifted bits. As these shifts are non-constant we can't regenerate the PointerInfo data with a fixed offset, so we need to discard the data entirely.

Fixes llvm#166744 - post-ra has to reconstruct dependencies after the chains have been stripped and uses pointer info instead - which resulted in some loads being rescheduled earlier than the dependent store as it was thought they didn't alias
@RKSimon RKSimon enabled auto-merge (squash) November 6, 2025 11:36
@llvmbot
Copy link
Member

llvmbot commented Nov 6, 2025

@llvm/pr-subscribers-backend-x86

Author: Simon Pilgrim (RKSimon)

Changes

#166337 replaces large (illegal type) loads/stores with a smaller i32 load/store based off the demanded shifted bits. As these shifts are non-constant we can't regenerate the PointerInfo data with a fixed offset, so we need to discard the data entirely.

Fixes #166744 - post-ra has to reconstruct dependencies after the chains have been stripped and uses pointer info instead - which resulted in some loads being rescheduled earlier than the dependent store as it was thought they didn't alias


Full diff: https://github.com/llvm/llvm-project/pull/166752.diff

2 Files Affected:

  • (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+4-2)
  • (modified) llvm/test/CodeGen/X86/pr166744.ll (+4-4)
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 4d44227b3ecd4..168e04109a0a5 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -53442,7 +53442,8 @@ static SDValue narrowBitOpRMW(StoreSDNode *St, const SDLoc &DL,
   }
 
   SDValue NewStore =
-      DAG.getStore(St->getChain(), DL, Res, NewPtr, St->getPointerInfo(),
+      DAG.getStore(St->getChain(), DL, Res, NewPtr,
+                   MachinePointerInfo(St->getPointerInfo().getAddrSpace()),
                    Align(), St->getMemOperand()->getFlags());
 
   // If there are other uses of StoredVal, replace with a new load of the
@@ -54639,7 +54640,8 @@ static SDValue combineTruncate(SDNode *N, SelectionDAG &DAG,
         SDValue NewPtr = DAG.getMemBasePlusOffset(
             Ld->getBasePtr(), PtrByteOfs, DL, SDNodeFlags::NoUnsignedWrap);
         SDValue NewLoad =
-            DAG.getLoad(VT, DL, Ld->getChain(), NewPtr, Ld->getPointerInfo(),
+            DAG.getLoad(VT, DL, Ld->getChain(), NewPtr,
+                        MachinePointerInfo(Ld->getPointerInfo().getAddrSpace()),
                         Align(), Ld->getMemOperand()->getFlags());
         DAG.makeEquivalentMemoryOrdering(Ld, NewLoad);
         return NewLoad;
diff --git a/llvm/test/CodeGen/X86/pr166744.ll b/llvm/test/CodeGen/X86/pr166744.ll
index de2a5e6cc78cc..21b25d87796a5 100644
--- a/llvm/test/CodeGen/X86/pr166744.ll
+++ b/llvm/test/CodeGen/X86/pr166744.ll
@@ -3,7 +3,7 @@
 ; RUN: llc < %s -mtriple=x86_64-- -mcpu=haswell | FileCheck %s --check-prefixes=NOPOSTRA
 ; RUN: llc < %s -mtriple=x86_64-- -mcpu=x86-64-v3 | FileCheck %s --check-prefixes=NOPOSTRA
 
-; FIXME: Ensure reloads are after narrowed i512 -> i32 store
+; Ensure reloads are after narrowed i512 -> i32 store
 define i1 @PR166744(ptr %v, i64 %idx, i1 zeroext %b) {
 ; POSTRA-LABEL: PR166744:
 ; POSTRA:       # %bb.0:
@@ -12,15 +12,15 @@ define i1 @PR166744(ptr %v, i64 %idx, i1 zeroext %b) {
 ; POSTRA-NEXT:    bextrl %eax, %esi, %eax
 ; POSTRA-NEXT:    movl (%rdi,%rax,4), %ecx
 ; POSTRA-NEXT:    btrl %esi, %ecx
-; POSTRA-NEXT:    movq 8(%rdi), %rsi
-; POSTRA-NEXT:    orq 40(%rdi), %rsi
 ; POSTRA-NEXT:    orl %ecx, %edx
 ; POSTRA-NEXT:    movl %edx, (%rdi,%rax,4)
 ; POSTRA-NEXT:    movq 16(%rdi), %rax
+; POSTRA-NEXT:    movq (%rdi), %rcx
 ; POSTRA-NEXT:    movq 24(%rdi), %rdx
+; POSTRA-NEXT:    movq 8(%rdi), %rsi
 ; POSTRA-NEXT:    orq 56(%rdi), %rdx
+; POSTRA-NEXT:    orq 40(%rdi), %rsi
 ; POSTRA-NEXT:    orq 48(%rdi), %rax
-; POSTRA-NEXT:    movq (%rdi), %rcx
 ; POSTRA-NEXT:    orq 32(%rdi), %rcx
 ; POSTRA-NEXT:    orq %rdx, %rsi
 ; POSTRA-NEXT:    orq %rax, %rcx

@RKSimon RKSimon merged commit 0663710 into llvm:main Nov 6, 2025
11 of 12 checks passed
@RKSimon RKSimon deleted the x86-bittest-ptrinfo branch November 6, 2025 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[X86] narrowBitOpRMW - post-ra enabled targets not retaining store-load dependency order on

2 participants