Skip to content

[ARM] Return the correct chain when expanding READ_REGISTER #145237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

davemgreen
Copy link
Collaborator

This prevents it CSEing multiple nodes together from "volatile" registers as they would end up with the same chain. The new chain out should be the chain from the new READ_REGISTER node.

Fixes #144845

This prevents it CSEing multiple nodes together from "volatile" registers as
they would end up with the same chain. The new chain out should be the chain
from the new READ_REGISTER node.

Fixes llvm#144845
@llvmbot
Copy link
Member

llvmbot commented Jun 22, 2025

@llvm/pr-subscribers-backend-arm

Author: David Green (davemgreen)

Changes

This prevents it CSEing multiple nodes together from "volatile" registers as they would end up with the same chain. The new chain out should be the chain from the new READ_REGISTER node.

Fixes #144845


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

2 Files Affected:

  • (modified) llvm/lib/Target/ARM/ARMISelLowering.cpp (+1-1)
  • (modified) llvm/test/CodeGen/ARM/special-reg.ll (+8-4)
diff --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp b/llvm/lib/Target/ARM/ARMISelLowering.cpp
index 4567081fe78dc..9cb2235d2c196 100644
--- a/llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -6192,7 +6192,7 @@ static void ExpandREAD_REGISTER(SDNode *N, SmallVectorImpl<SDValue> &Results,
 
   Results.push_back(DAG.getNode(ISD::BUILD_PAIR, DL, MVT::i64, Read.getValue(0),
                     Read.getValue(1)));
-  Results.push_back(Read.getOperand(0));
+  Results.push_back(Read.getValue(2)); // Chain
 }
 
 /// \p BC is a bitcast that is about to be turned into a VMOVDRR.
diff --git a/llvm/test/CodeGen/ARM/special-reg.ll b/llvm/test/CodeGen/ARM/special-reg.ll
index e966550e673d4..cc95f79d2c73b 100644
--- a/llvm/test/CodeGen/ARM/special-reg.ll
+++ b/llvm/test/CodeGen/ARM/special-reg.ll
@@ -25,14 +25,18 @@ entry:
 define i64 @read_volatile_i64_twice() {
 ; ACORE-LABEL: read_volatile_i64_twice:
 ; ACORE:       @ %bb.0: @ %entry
-; ACORE-NEXT:    mov r0, #0
-; ACORE-NEXT:    mov r1, #0
+; ACORE-NEXT:    mrrc p15, #1, r0, r1, c14
+; ACORE-NEXT:    mrrc p15, #1, r2, r3, c14
+; ACORE-NEXT:    eor r0, r2, r0
+; ACORE-NEXT:    eor r1, r3, r1
 ; ACORE-NEXT:    bx lr
 ;
 ; MCORE-LABEL: read_volatile_i64_twice:
 ; MCORE:       @ %bb.0: @ %entry
-; MCORE-NEXT:    movs r0, #0
-; MCORE-NEXT:    movs r1, #0
+; MCORE-NEXT:    mrrc p15, #1, r0, r1, c14
+; MCORE-NEXT:    mrrc p15, #1, r2, r3, c14
+; MCORE-NEXT:    eors r0, r2
+; MCORE-NEXT:    eors r1, r3
 ; MCORE-NEXT:    bx lr
 entry:
   %0 = tail call i64 @llvm.read_volatile_register.i64(metadata !5)

Copy link
Collaborator

@ostannard ostannard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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.

__arm_rsr64 treated as CSE'able on arm32
4 participants