Skip to content
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

[MachineSink] Some more preserving of debug location when rematerialising an instruction to replace a COPY #73155

Merged
merged 2 commits into from
Nov 24, 2023

Conversation

momchil-velikov
Copy link
Collaborator

@momchil-velikov momchil-velikov commented Nov 22, 2023

Somewhet similar to ef9bcac
([MachineSink][AArch64] Preserve debug location when rematerialising
an instruction to replace a COPY (#72685))
reuse the debug location of the COPY, iff the rematerialised instruction
did not have a location.

Fixes a regression in DebugInfo/AArch64/constant-dbgloc.ll after enabling sinj-and-fold.

…sing an instruction to replace a COPY

Somewhet similar to ef9bcac
([MachineSink][AArch64] Preserve debug location when rematerialising
 an instruction to replace a COPY (llvm#72685))
reuse the debug location of the COPY, iff the rematerialised instruction
did not have a location.
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 22, 2023

@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-backend-aarch64

Author: Momchil Velikov (momchil-velikov)

Changes

Somewhet similar to ef9bcac
([MachineSink][AArch64] Preserve debug location when rematerialising
an instruction to replace a COPY (#72685))
reuse the debug location of the COPY, iff the rematerialised instruction
did not have a location.


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

3 Files Affected:

  • (modified) llvm/lib/CodeGen/MachineSink.cpp (+3-1)
  • (modified) llvm/lib/Target/AArch64/AArch64TargetMachine.cpp (+1-1)
  • (modified) llvm/test/CodeGen/AArch64/sink-and-fold.ll (+1-1)
diff --git a/llvm/lib/CodeGen/MachineSink.cpp b/llvm/lib/CodeGen/MachineSink.cpp
index 48f5a21406f08a8..83d775055dfd733 100644
--- a/llvm/lib/CodeGen/MachineSink.cpp
+++ b/llvm/lib/CodeGen/MachineSink.cpp
@@ -506,7 +506,7 @@ bool MachineSinking::PerformSinkAndFold(MachineInstr &MI,
     MRI->clearKillFlags(UsedRegB);
 
   for (auto &[SinkDst, MaybeAM] : SinkInto) {
-    [[maybe_unused]] MachineInstr *New = nullptr;
+    MachineInstr *New = nullptr;
     LLVM_DEBUG(dbgs() << "Sinking copy of"; MI.dump(); dbgs() << "into";
                SinkDst->dump());
     if (SinkDst->isCopy()) {
@@ -525,6 +525,8 @@ bool MachineSinking::PerformSinkAndFold(MachineInstr &MI,
       Register DstReg = SinkDst->getOperand(0).getReg();
       TII->reMaterialize(*SinkDst->getParent(), InsertPt, DstReg, 0, MI, *TRI);
       New = &*std::prev(InsertPt);
+      if (!New->getDebugLoc())
+        New->setDebugLoc(SinkDst->getDebugLoc());
     } else {
       // Fold instruction into the addressing mode of a memory instruction.
       New = TII->emitLdStWithAddr(*SinkDst, MaybeAM);
diff --git a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
index d418a297218eb06..036719be06d83f1 100644
--- a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
+++ b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
@@ -200,7 +200,7 @@ static cl::opt<bool> EnableGISelLoadStoreOptPostLegal(
 static cl::opt<bool>
     EnableSinkFold("aarch64-enable-sink-fold",
                    cl::desc("Enable sinking and folding of instruction copies"),
-                   cl::init(false), cl::Hidden);
+                   cl::init(true), cl::Hidden);
 
 extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeAArch64Target() {
   // Register the target.
diff --git a/llvm/test/CodeGen/AArch64/sink-and-fold.ll b/llvm/test/CodeGen/AArch64/sink-and-fold.ll
index 632fdb391053121..52007221e12a7b5 100644
--- a/llvm/test/CodeGen/AArch64/sink-and-fold.ll
+++ b/llvm/test/CodeGen/AArch64/sink-and-fold.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -aarch64-enable-sink-fold=true < %s | FileCheck %s
+; RUN: llc < %s | FileCheck %s
 target triple = "aarch64-linux"
 
 declare i32 @use(...)

@davemgreen
Copy link
Collaborator

Can you add the sinkand-fold flag temporarily to DebugInfo/AArch64/constant-dbgloc.ll, to show it still working? It can be removed again later but helps show this is tested.

Copy link
Collaborator

@davemgreen davemgreen left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM

@momchil-velikov momchil-velikov merged commit 6b87d84 into llvm:main Nov 24, 2023
4 checks passed
@momchil-velikov momchil-velikov deleted the reuse-dbg-loc-next branch January 30, 2024 10:35
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.

None yet

3 participants