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

[RemoveDIs] Update Clang front end to handle DbgRecords #84756

Merged
merged 3 commits into from
Mar 18, 2024

Conversation

OCHyams
Copy link
Contributor

@OCHyams OCHyams commented Mar 11, 2024

This patch fixes problems that pop up when clang emits DbgRecords instead of debug intrinsics.

Note: this doesn't mean clang is emitting DbgRecords yet, because the modules it creates are still always in the old debug mode. That will come in a future patch.

Depends on #84739

fixes debug-info-blocks.m in both modes
Fixed OpenMP/debug_task_shared.c
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen labels Mar 11, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 11, 2024

@llvm/pr-subscribers-clang

Author: Orlando Cazalet-Hyams (OCHyams)

Changes

This patch fixes problems that pop up when clang emits DbgRecords instead of debug intrinsics.

Note: this doesn't mean clang is emitting DbgRecords yet, because the modules it creates are still always in the old debug mode. That will come in a future patch.

Depends on #84739


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

3 Files Affected:

  • (modified) clang/lib/CodeGen/CGBlocks.cpp (+4-1)
  • (modified) clang/lib/CodeGen/CGStmtOpenMP.cpp (+17-5)
  • (modified) clang/test/CodeGenObjC/debug-info-blocks.m (+1-1)
diff --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 0cbace7b7f7bbd..768b786c6426df 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -1540,7 +1540,10 @@ llvm::Function *CodeGenFunction::GenerateBlockFunction(
   llvm::BasicBlock *resume = Builder.GetInsertBlock();
 
   // Go back to the entry.
-  ++entry_ptr;
+  if (entry_ptr->getNextNonDebugInstruction())
+    entry_ptr = entry_ptr->getNextNonDebugInstruction()->getIterator();
+  else 
+    entry_ptr = entry->end();
   Builder.SetInsertPoint(entry, entry_ptr);
 
   // Emit debug information for all the DeclRefExprs.
diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index 3fbd2e03eb61df..5c6a3b8e001730 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -4746,10 +4746,10 @@ void CodeGenFunction::EmitOMPTaskBasedDirective(
           if (CGF.CGM.getCodeGenOpts().hasReducedDebugInfo())
             (void)DI->EmitDeclareOfAutoVariable(SharedVar, ContextValue,
                                                 CGF.Builder, false);
-          llvm::Instruction &Last = CGF.Builder.GetInsertBlock()->back();
           // Get the call dbg.declare instruction we just created and update
           // its DIExpression to add offset to base address.
-          if (auto DDI = dyn_cast<llvm::DbgVariableIntrinsic>(&Last)) {
+          auto UpdateExpr = [](llvm::LLVMContext &Ctx, auto *DDI,
+                               unsigned Offset) {
             SmallVector<uint64_t, 8> Ops;
             // Add offset to the base address if non zero.
             if (Offset) {
@@ -4757,9 +4757,21 @@ void CodeGenFunction::EmitOMPTaskBasedDirective(
               Ops.push_back(Offset);
             }
             Ops.push_back(llvm::dwarf::DW_OP_deref);
-            auto &Ctx = DDI->getContext();
-            llvm::DIExpression *DIExpr = llvm::DIExpression::get(Ctx, Ops);
-            Last.setOperand(2, llvm::MetadataAsValue::get(Ctx, DIExpr));
+            DDI->setExpression(llvm::DIExpression::get(Ctx, Ops));
+          };
+          llvm::Instruction &Last = CGF.Builder.GetInsertBlock()->back();
+          if (auto DDI = dyn_cast<llvm::DbgVariableIntrinsic>(&Last))
+            UpdateExpr(DDI->getContext(), DDI, Offset);
+          // If we're emitting using the new debug info format into a block
+          // without a terminator, the record will be "trailing".
+          assert(!Last.isTerminator() && "unexpected terminator");
+          if (auto *Marker =
+                  CGF.Builder.GetInsertBlock()->getTrailingDPValues()) {
+            for (llvm::DPValue &DPV : llvm::reverse(
+                     llvm::DPValue::filter(Marker->getDbgValueRange()))) {
+              UpdateExpr(Last.getContext(), &DPV, Offset);
+              break;
+            }
           }
         }
       }
diff --git a/clang/test/CodeGenObjC/debug-info-blocks.m b/clang/test/CodeGenObjC/debug-info-blocks.m
index 14b29f222fbe8e..59171da016da1e 100644
--- a/clang/test/CodeGenObjC/debug-info-blocks.m
+++ b/clang/test/CodeGenObjC/debug-info-blocks.m
@@ -5,8 +5,8 @@
 
 // CHECK: define {{.*}}_block_invoke
 // CHECK: store ptr %.block_descriptor, ptr %[[ALLOCA:block.addr]], align
-// CHECK-NEXT: call void @llvm.dbg.declare(metadata ptr %[[ALLOCA]], metadata ![[SELF:[0-9]+]], metadata !{{.*}})
 // CHECK-NEXT: call void @llvm.dbg.declare(metadata ptr %d, metadata ![[D:[0-9]+]], metadata !{{.*}})
+// CHECK-NEXT: call void @llvm.dbg.declare(metadata ptr %[[ALLOCA]], metadata ![[SELF:[0-9]+]], metadata !{{.*}})
 
 // Test that we do emit scope info for the helper functions, and that the
 // parameters to these functions are marked as artificial (so the debugger

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 11, 2024

@llvm/pr-subscribers-clang-codegen

Author: Orlando Cazalet-Hyams (OCHyams)

Changes

This patch fixes problems that pop up when clang emits DbgRecords instead of debug intrinsics.

Note: this doesn't mean clang is emitting DbgRecords yet, because the modules it creates are still always in the old debug mode. That will come in a future patch.

Depends on #84739


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

3 Files Affected:

  • (modified) clang/lib/CodeGen/CGBlocks.cpp (+4-1)
  • (modified) clang/lib/CodeGen/CGStmtOpenMP.cpp (+17-5)
  • (modified) clang/test/CodeGenObjC/debug-info-blocks.m (+1-1)
diff --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 0cbace7b7f7bbd..768b786c6426df 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -1540,7 +1540,10 @@ llvm::Function *CodeGenFunction::GenerateBlockFunction(
   llvm::BasicBlock *resume = Builder.GetInsertBlock();
 
   // Go back to the entry.
-  ++entry_ptr;
+  if (entry_ptr->getNextNonDebugInstruction())
+    entry_ptr = entry_ptr->getNextNonDebugInstruction()->getIterator();
+  else 
+    entry_ptr = entry->end();
   Builder.SetInsertPoint(entry, entry_ptr);
 
   // Emit debug information for all the DeclRefExprs.
diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index 3fbd2e03eb61df..5c6a3b8e001730 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -4746,10 +4746,10 @@ void CodeGenFunction::EmitOMPTaskBasedDirective(
           if (CGF.CGM.getCodeGenOpts().hasReducedDebugInfo())
             (void)DI->EmitDeclareOfAutoVariable(SharedVar, ContextValue,
                                                 CGF.Builder, false);
-          llvm::Instruction &Last = CGF.Builder.GetInsertBlock()->back();
           // Get the call dbg.declare instruction we just created and update
           // its DIExpression to add offset to base address.
-          if (auto DDI = dyn_cast<llvm::DbgVariableIntrinsic>(&Last)) {
+          auto UpdateExpr = [](llvm::LLVMContext &Ctx, auto *DDI,
+                               unsigned Offset) {
             SmallVector<uint64_t, 8> Ops;
             // Add offset to the base address if non zero.
             if (Offset) {
@@ -4757,9 +4757,21 @@ void CodeGenFunction::EmitOMPTaskBasedDirective(
               Ops.push_back(Offset);
             }
             Ops.push_back(llvm::dwarf::DW_OP_deref);
-            auto &Ctx = DDI->getContext();
-            llvm::DIExpression *DIExpr = llvm::DIExpression::get(Ctx, Ops);
-            Last.setOperand(2, llvm::MetadataAsValue::get(Ctx, DIExpr));
+            DDI->setExpression(llvm::DIExpression::get(Ctx, Ops));
+          };
+          llvm::Instruction &Last = CGF.Builder.GetInsertBlock()->back();
+          if (auto DDI = dyn_cast<llvm::DbgVariableIntrinsic>(&Last))
+            UpdateExpr(DDI->getContext(), DDI, Offset);
+          // If we're emitting using the new debug info format into a block
+          // without a terminator, the record will be "trailing".
+          assert(!Last.isTerminator() && "unexpected terminator");
+          if (auto *Marker =
+                  CGF.Builder.GetInsertBlock()->getTrailingDPValues()) {
+            for (llvm::DPValue &DPV : llvm::reverse(
+                     llvm::DPValue::filter(Marker->getDbgValueRange()))) {
+              UpdateExpr(Last.getContext(), &DPV, Offset);
+              break;
+            }
           }
         }
       }
diff --git a/clang/test/CodeGenObjC/debug-info-blocks.m b/clang/test/CodeGenObjC/debug-info-blocks.m
index 14b29f222fbe8e..59171da016da1e 100644
--- a/clang/test/CodeGenObjC/debug-info-blocks.m
+++ b/clang/test/CodeGenObjC/debug-info-blocks.m
@@ -5,8 +5,8 @@
 
 // CHECK: define {{.*}}_block_invoke
 // CHECK: store ptr %.block_descriptor, ptr %[[ALLOCA:block.addr]], align
-// CHECK-NEXT: call void @llvm.dbg.declare(metadata ptr %[[ALLOCA]], metadata ![[SELF:[0-9]+]], metadata !{{.*}})
 // CHECK-NEXT: call void @llvm.dbg.declare(metadata ptr %d, metadata ![[D:[0-9]+]], metadata !{{.*}})
+// CHECK-NEXT: call void @llvm.dbg.declare(metadata ptr %[[ALLOCA]], metadata ![[SELF:[0-9]+]], metadata !{{.*}})
 
 // Test that we do emit scope info for the helper functions, and that the
 // parameters to these functions are marked as artificial (so the debugger

Copy link

github-actions bot commented Mar 11, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff e77f5fe889909df3508fd929f2636a0ac211877a 79fc0ea1a68c623c1207509ab00f3bd8a6c16961 -- clang/lib/CodeGen/CGBlocks.cpp clang/lib/CodeGen/CGStmtOpenMP.cpp
View the diff from clang-format here.
diff --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 768b786c64..ad0b50d799 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -1542,7 +1542,7 @@ llvm::Function *CodeGenFunction::GenerateBlockFunction(
   // Go back to the entry.
   if (entry_ptr->getNextNonDebugInstruction())
     entry_ptr = entry_ptr->getNextNonDebugInstruction()->getIterator();
-  else 
+  else
     entry_ptr = entry->end();
   Builder.SetInsertPoint(entry, entry_ptr);
 

@OCHyams OCHyams requested review from SLTozer and jryans March 11, 2024 14:39
Copy link
Contributor

@SLTozer SLTozer left a comment

Choose a reason for hiding this comment

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

Maybe wants someone with more frontend knowledge to chime in, but all SGTM.

// Get the call dbg.declare instruction we just created and update
// its DIExpression to add offset to base address.
if (auto DDI = dyn_cast<llvm::DbgVariableIntrinsic>(&Last)) {
auto UpdateExpr = [](llvm::LLVMContext &Ctx, auto *DDI,
Copy link
Contributor

Choose a reason for hiding this comment

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

Super nit, but in situations like this I prefer to change things like DDI to Declare; subjective, so non-blocking.

@OCHyams
Copy link
Contributor Author

OCHyams commented Mar 12, 2024

Maybe wants someone with more frontend knowledge to chime in, but all SGTM.

@adrian-prantl are you able to suggest anyone that might be able to take a look at this (part of this touches some Objective-C code+test) - we're not overly familiar with front end code.

Copy link
Contributor

@SLTozer SLTozer left a comment

Choose a reason for hiding this comment

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

It's been a bit without any comments, probably safe now to land this.

@OCHyams OCHyams merged commit 6f60ad7 into llvm:main Mar 18, 2024
2 of 4 checks passed
OCHyams added a commit that referenced this pull request Mar 18, 2024
OCHyams added a commit that referenced this pull request Mar 18, 2024
OCHyams added a commit that referenced this pull request Mar 18, 2024
This patch fixes problems that pop up when clang emits DbgRecords
instead of debug intrinsics.

Note: this doesn't mean clang is emitting DbgRecords yet, because the
modules it creates are still always in the old debug mode. That will
come in a future patch.

Depends on #84739
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants