[clang] [RemoveDIs] Update Clang front end to handle DbgRecords (PR #84756)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams closed 
https://github.com/llvm/llvm-project/pull/84756
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] b097b3d - Fix formatting in #84756

[Orlando Cazalet-Hyams via cfe-commits]

Author: Orlando Cazalet-Hyams
Date: 2024-03-18T10:01:51Z
New Revision: b097b3dc2ba2517621a5e3da3237a77ed0e7586f

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

LOG: Fix formatting in #84756

Added: 


Modified: 
clang/lib/CodeGen/CGBlocks.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 768b786c6426df..ad0b50d799618e 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);
 



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 3e4170a - Revert "Fix formatting in #84756"

[Orlando Cazalet-Hyams via cfe-commits]

Author: Orlando Cazalet-Hyams
Date: 2024-03-18T10:13:12Z
New Revision: 3e4170a587adb789b77ede799d09139b50ebe5bc

URL: 
https://github.com/llvm/llvm-project/commit/3e4170a587adb789b77ede799d09139b50ebe5bc
DIFF: 
https://github.com/llvm/llvm-project/commit/3e4170a587adb789b77ede799d09139b50ebe5bc.diff

LOG: Revert "Fix formatting in #84756"

This reverts commit b097b3dc2ba2517621a5e3da3237a77ed0e7586f.

Buildbots: https://lab.llvm.org/buildbot/#/builders/196/builds/47206

Added: 


Modified: 
clang/lib/CodeGen/CGBlocks.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index ad0b50d799618e..768b786c6426df 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);
 



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 92122b0 - Revert "[RemoveDIs] Update Clang front end to handle DbgRecords (#84756)"

[Orlando Cazalet-Hyams via cfe-commits]

Author: Orlando Cazalet-Hyams
Date: 2024-03-18T10:13:35Z
New Revision: 92122b0b4b514ea6c081e428f47ef1bf9d4f0f17

URL: 
https://github.com/llvm/llvm-project/commit/92122b0b4b514ea6c081e428f47ef1bf9d4f0f17
DIFF: 
https://github.com/llvm/llvm-project/commit/92122b0b4b514ea6c081e428f47ef1bf9d4f0f17.diff

LOG: Revert "[RemoveDIs] Update Clang front end to handle DbgRecords (#84756)"

This reverts commit 6f60ad7e9a3508f19d54c827cf11f7930a0685ee.

Buildbots: https://lab.llvm.org/buildbot/#/builders/196/builds/47206

Added: 


Modified: 
clang/lib/CodeGen/CGBlocks.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/test/CodeGenObjC/debug-info-blocks.m

Removed: 




diff  --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 768b786c6426df..0cbace7b7f7bbd 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -1540,10 +1540,7 @@ llvm::Function *CodeGenFunction::GenerateBlockFunction(
   llvm::BasicBlock *resume = Builder.GetInsertBlock();
 
   // Go back to the entry.
-  if (entry_ptr->getNextNonDebugInstruction())
-entry_ptr = entry_ptr->getNextNonDebugInstruction()->getIterator();
-  else 
-entry_ptr = entry->end();
+  ++entry_ptr;
   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 d7c18146b71b16..452ce6983f6ac1 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -4749,10 +4749,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.
-  auto UpdateExpr = [](llvm::LLVMContext &Ctx, auto *Declare,
-   unsigned Offset) {
+  if (auto DDI = dyn_cast(&Last)) {
 SmallVector Ops;
 // Add offset to the base address if non zero.
 if (Offset) {
@@ -4760,21 +4760,9 @@ void CodeGenFunction::EmitOMPTaskBasedDirective(
   Ops.push_back(Offset);
 }
 Ops.push_back(llvm::dwarf::DW_OP_deref);
-Declare->setExpression(llvm::DIExpression::get(Ctx, Ops));
-  };
-  llvm::Instruction &Last = CGF.Builder.GetInsertBlock()->back();
-  if (auto DDI = dyn_cast(&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;
-}
+auto &Ctx = DDI->getContext();
+llvm::DIExpression *DIExpr = llvm::DIExpression::get(Ctx, Ops);
+Last.setOperand(2, llvm::MetadataAsValue::get(Ctx, DIExpr));
   }
 }
   }

diff  --git a/clang/test/CodeGenObjC/debug-info-blocks.m 
b/clang/test/CodeGenObjC/debug-info-blocks.m
index 59171da016da1e..14b29f222fbe8e 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 %d, metadata 
![[D:[0-9]+]], metadata !{{.*}})
 // 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 !{{.*}})
 
 // 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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 3e6db60 - [RemoveDIs] Update Clang front end to handle DbgRecords (#84756)

[Orlando Cazalet-Hyams via cfe-commits]

Author: Orlando Cazalet-Hyams
Date: 2024-03-18T10:55:29Z
New Revision: 3e6db602918435b6a5ac476f63f8b259e7e73af4

URL: 
https://github.com/llvm/llvm-project/commit/3e6db602918435b6a5ac476f63f8b259e7e73af4
DIFF: 
https://github.com/llvm/llvm-project/commit/3e6db602918435b6a5ac476f63f8b259e7e73af4.diff

LOG: [RemoveDIs] Update Clang front end to handle DbgRecords (#84756)

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

Added: 


Modified: 
clang/lib/CodeGen/CGBlocks.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/test/CodeGenObjC/debug-info-blocks.m

Removed: 




diff  --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 0cbace7b7f7bbd..ad0b50d799618e 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 452ce6983f6ac1..8edcc4ceea9436 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -4749,10 +4749,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(&Last)) {
+  auto UpdateExpr = [](llvm::LLVMContext &Ctx, auto *Declare,
+   unsigned Offset) {
 SmallVector Ops;
 // Add offset to the base address if non zero.
 if (Offset) {
@@ -4760,9 +4760,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));
+Declare->setExpression(llvm::DIExpression::get(Ctx, Ops));
+  };
+  llvm::Instruction &Last = CGF.Builder.GetInsertBlock()->back();
+  if (auto DDI = dyn_cast(&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()->getTrailingDbgRecords()) {
+for (llvm::DPValue &DPV : llvm::reverse(
+ llvm::filterDbgVars(Marker->getDbgRecordRange( {
+  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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [RemoveDIs] Update Clang front end to handle DbgRecords (PR #84756)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams created 
https://github.com/llvm/llvm-project/pull/84756

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.



>From 8b37020a48b8145ad9d7deb288b9f8a3ee3a9a9b Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams 
Date: Sat, 9 Mar 2024 13:10:05 +
Subject: [PATCH 1/2] clang: insertbefore non-debug for removeDIs stability

fixes debug-info-blocks.m in both modes
---
 clang/lib/CodeGen/CGBlocks.cpp | 5 -
 clang/test/CodeGenObjC/debug-info-blocks.m | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

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/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

>From efabe4b06f38aaf3aed8cf02c97f566652ba5f15 Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams 
Date: Sat, 9 Mar 2024 13:40:30 +
Subject: [PATCH 2/2] openmp update DbgRecords

Fixed OpenMP/debug_task_shared.c
---
 clang/lib/CodeGen/CGStmtOpenMP.cpp | 22 +-
 1 file changed, 17 insertions(+), 5 deletions(-)

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(&Last)) {
+  auto UpdateExpr = [](llvm::LLVMContext &Ctx, auto *DDI,
+   unsigned Offset) {
 SmallVector 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(&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;
+}
   }
 }
   }

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [RemoveDIs] Update Clang front end to handle DbgRecords (PR #84756)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams edited 
https://github.com/llvm/llvm-project/pull/84756
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [RemoveDIs] Update Clang front end to handle DbgRecords (PR #84756)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams updated 
https://github.com/llvm/llvm-project/pull/84756

>From 8b37020a48b8145ad9d7deb288b9f8a3ee3a9a9b Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams 
Date: Sat, 9 Mar 2024 13:10:05 +
Subject: [PATCH 1/3] clang: insertbefore non-debug for removeDIs stability

fixes debug-info-blocks.m in both modes
---
 clang/lib/CodeGen/CGBlocks.cpp | 5 -
 clang/test/CodeGenObjC/debug-info-blocks.m | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

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/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

>From efabe4b06f38aaf3aed8cf02c97f566652ba5f15 Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams 
Date: Sat, 9 Mar 2024 13:40:30 +
Subject: [PATCH 2/3] openmp update DbgRecords

Fixed OpenMP/debug_task_shared.c
---
 clang/lib/CodeGen/CGStmtOpenMP.cpp | 22 +-
 1 file changed, 17 insertions(+), 5 deletions(-)

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(&Last)) {
+  auto UpdateExpr = [](llvm::LLVMContext &Ctx, auto *DDI,
+   unsigned Offset) {
 SmallVector 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(&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;
+}
   }
 }
   }

>From 79fc0ea1a68c623c1207509ab00f3bd8a6c16961 Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams 
Date: Mon, 11 Mar 2024 12:07:32 +
Subject: [PATCH 3/3] rename variable

---
 clang/lib/CodeGen/CGStmtOpenMP.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp 
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index 5c6a3b8e001730..20c80b6bc235e4 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/

[clang] [RemoveDIs] Update Clang front end to handle DbgRecords (PR #84756)

[Orlando Cazalet-Hyams via cfe-commits]

OCHyams wrote:

> 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.

https://github.com/llvm/llvm-project/pull/84756
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [DebugInfo][RemoveDIs] Handle non-instr debug-info in GlobalISel (PR #75228)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams approved this pull request.

LGTM 👍 

https://github.com/llvm/llvm-project/pull/75228
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][DebugInfo] Clang generates an extra spurious unnamed 'dbg.declare' (PR #69681)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams edited 
https://github.com/llvm/llvm-project/pull/69681
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][DebugInfo] Clang generates an extra spurious unnamed 'dbg.declare' (PR #69681)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams commented:

This SGTM.

I don't think the unnamed variable serves any useful purpose but may not being 
imaginative enough. I can't see any discussion on the topic when the code was 
added in [D119178](https://reviews.llvm.org/D119178) . CC the author, @shafik 
(or @adrian-prantl) - is there a use-case for the anonymous variable that 
covers the whole of the structured binding storage?

https://github.com/llvm/llvm-project/pull/69681
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][DebugInfo] Clang generates an extra spurious unnamed 'dbg.declare' (PR #69681)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -2,10 +2,8 @@
 

OCHyams wrote:

Please can you add `--implicit-check-not="call void @llvm.dbg.declare"` to the 
`FileCheck` command?

https://github.com/llvm/llvm-project/pull/69681
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][DebugInfo] Clang generates an extra spurious unnamed 'dbg.declare' (PR #69681)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone %s -o - | FileCheck 
%s
+
+struct Tuple {
+  int Fld_1;
+  int Fld_2;
+};
+__attribute__((optnone)) Tuple get() { return {10, 20}; }
+
+// CHECK-LABEL: define dso_local noundef i32 @main
+// CHECK:  %retval = alloca i32, align 4
+// CHECK-NEXT: [[T0:%.*]] = alloca %struct.Tuple, align 4
+// CHECK:  call void @llvm.dbg.declare(metadata ptr [[T0]], metadata 
{{.*}}, metadata !DIExpression())
+// CHECK:  call void @llvm.dbg.declare(metadata ptr [[T0]], metadata 
{{.*}}, metadata !DIExpression(DW_OP_plus_uconst, {{[0-9]+}}))
+// CHECK-NOT:  call void @llvm.dbg.declare(metadata ptr [[T0]], metadata 
{{.*}}, metadata !DIExpression())

OCHyams wrote:

It doesn't look like this test covers anything that 
`clang/test/CodeGenCXX/debug-info-structured-binding.cpp` doesn't already?

IMO no need to add this one. Please can you modify the other test to check that 
the DILocalVariables in the dbg.declares have the expected names (i.e., confirm 
there's no unnamed variable).

https://github.com/llvm/llvm-project/pull/69681
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][DebugInfo] Clang generates an extra spurious unnamed 'dbg.declare' (PR #69681)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams approved this pull request.

New changes LGTM, thanks!

https://github.com/llvm/llvm-project/pull/69681
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

OCHyams wrote:

> LLVM IR parts look good to me.

Thanks!

@pogo59 said:

> Converting frame-types.s from v4 to v5 should be done as a separate commit. 
> Then adding the alias would be a simpler change to review.

I've split that into two commits in this PR: upgrade to v5: 
[18446f2](https://github.com/llvm/llvm-project/pull/87623/commits/18446f27e31803057d9d90c957e5e191eb263b7b),
 add template alias: 
[49d6642](https://github.com/llvm/llvm-project/pull/87623/commits/49d66423ae61baf9cacd42c2b57333e09e4c1e81).
 It does improve readability a bit but its still quite noisy. I am happy to 
commit the former separately then rebase this PR, if you'd like? 

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] Remove remaining uses of Instruction-constructors that insert before another Instruction (PR #85981)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -1542,19 +1542,10 @@ class CallBase : public Instruction {
 OperandBundleDef OB,
 Instruction *InsertPt = nullptr);
 
-  /// Create a clone of \p CB with operand bundle \p OB added.
-  static CallBase *addOperandBundle(CallBase *CB, uint32_t ID,
-OperandBundleDef OB,
-BasicBlock::iterator InsertPt);
-
   /// Create a clone of \p CB with operand bundle \p ID removed.
   static CallBase *removeOperandBundle(CallBase *CB, uint32_t ID,
Instruction *InsertPt = nullptr);
 
-  /// Create a clone of \p CB with operand bundle \p ID removed.
-  static CallBase *removeOperandBundle(CallBase *CB, uint32_t ID,
-   BasicBlock::iterator InsertPt);

OCHyams wrote:

why are these removed?

https://github.com/llvm/llvm-project/pull/85981
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] Remove remaining uses of Instruction-constructors that insert before another Instruction (PR #85981)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams approved this pull request.

LGTM if this is clang-formatted (I can't tell). Plus one small inline nit - I 
think it's fine either way.

https://github.com/llvm/llvm-project/pull/85981
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] Remove remaining uses of Instruction-constructors that insert before another Instruction (PR #85981)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams edited 
https://github.com/llvm/llvm-project/pull/85981
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] Remove remaining uses of Instruction-constructors that insert before another Instruction (PR #85981)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -542,7 +542,8 @@ void InsertPHIStrategy::mutate(BasicBlock &BB, 
RandomIRBuilder &IB) {
   if (&BB == &BB.getParent()->getEntryBlock())
 return;
   Type *Ty = IB.randomType();
-  PHINode *PHI = PHINode::Create(Ty, llvm::pred_size(&BB), "", &BB.front());
+  PHINode *PHI =
+  PHINode::Create(Ty, llvm::pred_size(&BB), "", BB.getFirstInsertionPt());

OCHyams wrote:

`BB.begin()` instead of `BB.getFirstInsertionPt()`?

https://github.com/llvm/llvm-project/pull/85981
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -310,6 +310,23 @@ namespace llvm {
  DINode::DIFlags Flags = DINode::FlagZero,
  DINodeArray Annotations = nullptr);
 
+/// Create debugging information entry for a typedef.
+/// \param Ty  Original type.
+/// \param NameTypedef name.
+/// \param FileFile where this type is defined.
+/// \param LineNo  Line number.
+/// \param Context The surrounding context for the typedef.
+/// \param TParams The template arguments.
+/// \param AlignInBits Alignment. (optional)
+/// \param Flags   Flags to describe inheritance attribute, e.g. 
private

OCHyams wrote:

Fixed

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -5361,7 +5383,56 @@ static bool IsReconstitutableType(QualType QT) {
   return T.Reconstitutable;
 }
 
-std::string CGDebugInfo::GetName(const Decl *D, bool Qualified) const {
+bool CGDebugInfo::HasReconstitutableArgs(
+ArrayRef Args) const {
+  return llvm::all_of(Args, [&](const TemplateArgument &TA) {
+switch (TA.getKind()) {
+case TemplateArgument::Template:
+  // Easy to reconstitute - the value of the parameter in the debug
+  // info is the string name of the template. (so the template name
+  // itself won't benefit from any name rebuilding, but that's a
+  // representational limitation - maybe DWARF could be
+  // changed/improved to use some more structural representation)
+  return true;
+case TemplateArgument::Declaration:
+  // Reference and pointer non-type template parameters point to
+  // variables, functions, etc and their value is, at best (for
+  // variables) represented as an address - not a reference to the
+  // DWARF describing the variable/function/etc. This makes it hard,
+  // possibly impossible to rebuild the original name - looking up
+  // the address in the executable file's symbol table would be
+  // needed.
+  return false;
+case TemplateArgument::NullPtr:
+  // These could be rebuilt, but figured they're close enough to the
+  // declaration case, and not worth rebuilding.
+  return false;
+case TemplateArgument::Pack:
+  // A pack is invalid if any of the elements of the pack are
+  // invalid.
+  return HasReconstitutableArgs(TA.getPackAsArray());
+case TemplateArgument::Integral:
+  // Larger integers get encoded as DWARF blocks which are a bit
+  // harder to parse back into a large integer, etc - so punting on
+  // this for now. Re-parsing the integers back into APInt is
+  // probably feasible some day.
+  return TA.getAsIntegral().getBitWidth() <= 64 &&
+ IsReconstitutableType(TA.getIntegralType());
+case TemplateArgument::StructuralValue:
+  return false;
+case TemplateArgument::Type:
+  return IsReconstitutableType(TA.getAsType());
+case TemplateArgument::Expression:
+  return IsReconstitutableType(TA.getAsExpr()->getType());
+default:
+  llvm_unreachable("Other, unresolved, template arguments should "
+   "not be seen here");
+}
+  });
+}
+
+std::string CGDebugInfo::GetName(const Decl *D, bool Qualified,
+ const Type *Ty) const {

OCHyams wrote:

Aha, good catch. You're correct on both counts - it's not used in this patch 
and that was one of the approaches I tried out.

This is discussed a bit on the issue starting here 
https://github.com/llvm/llvm-project/issues/54624#issuecomment-2024754144.

IIRC, we _could_ pass in the `TemplateSpecializationType` to get the template 
arguments for this case, but I think we'd need to introduce a special case for 
building the name where we can't use `D`'s  `getNameForDiagnostic` on line 5461 
and 5481 of the original file.

My memory is slightly blurred by the conference - I think I was erring on the 
side of caution given my unfamiliarity with the code, preferring to have a 
special case localised at the usage site rather than inside a utility function. 
I'd be happy to change the patch to use the `TemplateSpecializationType` 
parameter if you'd like to see what it looks like?




https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -152,9 +152,11 @@ uint64_t DebugHandlerBase::getBaseTypeSize(const DIType 
*Ty) {
   unsigned Tag = DDTy->getTag();
 
   if (Tag != dwarf::DW_TAG_member && Tag != dwarf::DW_TAG_typedef &&
-  Tag != dwarf::DW_TAG_const_type && Tag != dwarf::DW_TAG_volatile_type &&
+  Tag != dwarf::DW_TAG_template_alias && Tag != dwarf::DW_TAG_const_type &&
+  Tag != dwarf::DW_TAG_volatile_type &&
   Tag != dwarf::DW_TAG_restrict_type && Tag != dwarf::DW_TAG_atomic_type &&
-  Tag != dwarf::DW_TAG_immutable_type)
+  Tag != dwarf::DW_TAG_immutable_type &&
+  Tag != dwarf::DW_TAG_template_alias)

OCHyams wrote:

Fixed

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

OCHyams wrote:

Fixed

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Extend EmitPseudoVariable to support debug records (PR #94956)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/94956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

OCHyams wrote:

Thanks @dwblaikie 

> Bit unfortunate to store template parameters in different ways (in the 
> extraData for the alias template, but in the templateParams for the composite 
> types) - but I guess it'd be more invasive to try to represent alias 
> templates as composite types?

I hadn't actually tried using DICompsiteType. I didn't choose DIDerivedType for 
any particularly principled reasons: typedefs use derived types which made it 
very easy to find places that needed updating, and an alias "felt" more like a 
derived type than composite type. Having looked only briefly, I don't _think_ 
it'd be any more invasive to use composite types instead (not 100% sure without 
diving in). I'm happy to give that a go if that is your preference?

---

I've just found an input that causes an assertion failure in 
`CollectTemplateParams` with my implementation:
```
template
struct X {
  Y m1;
  Z m2;
};

template
using A = X;

A a;
```

There's a mismatch between the template parameter list and template argument 
list sizes, created here:
```
  if (CGM.getCodeGenOpts().DebugTemplateAlias) {
TemplateArgs Args = {TD->getTemplateParameters(), 
Ty->template_arguments()}; // <--- here
 ```
 
I notice that  we emit `DW_TAG_GNU_template_parameter_pack` for, e.g., variadic 
template struct instantiations. I've not figured out how to fix this just yet, 
but thought I'd bring it up in case there's something obvious.

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

OCHyams wrote:

> But perhaps at least you could add named accessors to DIDerivedType for the 
> template params, same as DIGlobalVariable/DICompositeType have?

Done. I couldn't add an assert (`getTag() == dwarf::DW_TAG_template_alias`) 
without including `Dwarf.h`. I've chosen to not do that as it looks like its 
been avoided (but possibly just not really needed here before), but I'd be 
happy to add it if the extra compile time isn't worth worrying about.

> (oh, and in case anyone hasn't mentioned it already - this would generally be 
> committed in smaller pieces upstream - adding the LLVM functionality first, 
> then adding clang patches that use that functionality)

Sure, no problem, I'll open separate pull requests once this has been accepted 
then.

---

I think I've addressed all the concerns raised now except for the variadic 
issue I mentioned, which I'll look at tomorrow.

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -5361,7 +5383,56 @@ static bool IsReconstitutableType(QualType QT) {
   return T.Reconstitutable;
 }
 
-std::string CGDebugInfo::GetName(const Decl *D, bool Qualified) const {
+bool CGDebugInfo::HasReconstitutableArgs(
+ArrayRef Args) const {
+  return llvm::all_of(Args, [&](const TemplateArgument &TA) {
+switch (TA.getKind()) {
+case TemplateArgument::Template:
+  // Easy to reconstitute - the value of the parameter in the debug
+  // info is the string name of the template. (so the template name
+  // itself won't benefit from any name rebuilding, but that's a
+  // representational limitation - maybe DWARF could be
+  // changed/improved to use some more structural representation)

OCHyams wrote:

👍 Promoted the sentence out of parens.

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -4584,6 +4584,32 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, 
const llvm::Triple &T,
 }
   }
 
+  // Emit DW_TAG_template_alias for template aliases? True by default for SCE.
+  const auto *DebugTemplateAlias = Args.getLastArg(
+  options::OPT_gtemplate_alias, options::OPT_gno_template_alias);
+  bool UseDebugTemplateAlias =
+  DebuggerTuning == llvm::DebuggerKind::SCE && RequestedDWARFVersion >= 5;
+  if (DebugTemplateAlias &&
+  checkDebugInfoOption(DebugTemplateAlias, Args, D, TC)) {
+const auto &Opt = DebugTemplateAlias->getOption();
+UseDebugTemplateAlias = Opt.matches(options::OPT_gtemplate_alias);
+  }
+  if (UseDebugTemplateAlias) {
+// DW_TAG_template_alias is a DWARFv5 feature. Warn if we can't use it.
+if (DebugTemplateAlias && RequestedDWARFVersion < 5)
+  D.Diag(diag::warn_drv_dwarf_feature_requires_version)
+  << DebugTemplateAlias->getAsString(Args) << 5
+  << RequestedDWARFVersion;
+else if (DebugTemplateAlias && EffectiveDWARFVersion < 5)
+  // The toolchain has reduced allowed dwarf version, so we can't enable
+  // -gtemplate-alias.
+  D.Diag(diag::warn_drv_dwarf_version_limited_by_target)
+  << DebugTemplateAlias->getAsString(Args) << TC.getTripleString() << 5
+  << EffectiveDWARFVersion;
+else
+  CmdArgs.push_back("-gtemplate-alias");
+  }

OCHyams wrote:

Done

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -465,3 +465,15 @@
 // MANGLED_TEMP_NAMES: error: unknown argument 
'-gsimple-template-names=mangled'; did you mean '-Xclang 
-gsimple-template-names=mangled'
 // RUN: %clang -### -target x86_64 -c -g %s 2>&1 | FileCheck 
--check-prefix=FULL_TEMP_NAMES 
--implicit-check-not=debug-forward-template-params %s
 // FULL_TEMP_NAMES-NOT: -gsimple-template-names
+
+ Test -g[no-]template-alias (enabled by default with SCE debugger tuning). 
Requires DWARFv5.
+// RUN: %clang -### -target x86_64 -c -gdwarf-5 -gsce %s 2>&1 | FileCheck %s 
--check-prefixes=TEMPLATE-ALIAS

OCHyams wrote:

This gave `clang: error: unknown argument '--target'; did you mean '-target'?`

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -1,4 +1,4 @@
-// RUN: %clang -g -std=c++11 -S -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang -ggdb -std=c++11 -S -emit-llvm %s -o - | FileCheck %s

OCHyams wrote:

Agreed, done

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams edited 
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const 
TemplateSpecializationType *Ty,
   auto PP = getPrintingPolicy();
   Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
 
+  SourceLocation Loc = AliasDecl->getLocation();
+
+  if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substitued into a parameter pack. We can find out if that's

OCHyams wrote:

I need a clang-format-spellcheck or something. Thanks, fixed.

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams commented:

(oops, didn't submit my inline replies...)

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -310,6 +310,24 @@ namespace llvm {
  DINode::DIFlags Flags = DINode::FlagZero,
  DINodeArray Annotations = nullptr);
 
+/// Create debugging information entry for a template alias.
+/// \param Ty  Original type.
+/// \param NameAlias name.
+/// \param FileFile where this type is defined.
+/// \param LineNo  Line number.
+/// \param Context The surrounding context for the alias.
+/// \param TParams The template arguments.
+/// \param AlignInBits Alignment. (optional)
+/// \param Flags   Flags to describe inheritance attribute (optional),
+///e.g. private.

OCHyams wrote:

It says so at the end of the line. I suppose it would be more in keeping if it 
came after the full stop `e.g. private. (here)`, but it felt right where it is 
at the moment too. ymmv.

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const 
TemplateSpecializationType *Ty,
   auto PP = getPrintingPolicy();
   Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
 
+  SourceLocation Loc = AliasDecl->getLocation();
+
+  if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+  ArrayRef SubstArgs = Ty->template_arguments();
+  for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+  SpecArgs.push_back(TemplateArgument(SubstArgs));
+  break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+  // If SubstArgs is now empty (we're taking from it each iteration) 
and
+  // this template parameter isn't a pack, then that should mean we're
+  // using default values for the remaining template parameters.
+  break;

OCHyams wrote:

> Does this mean we won't be emitting the defaulted template arguments as part 
> of the DW_TAG_template_alias?

Yes. At the moment (without this patch, with typedefs) the names get 
constructed without the default values which is different behaviour to structs:
```
template
struct X {
  char n;
};

template
using B = X;

// DW_TAG_typedef
//   DW_AT_name  ("B<>")"
B<> a;

// DW_TAG_typedef
//   DW_AT_name  ("B")
B b;


template
struct F {
  char n;
};

// DW_TAG_structure_type
//   DW_AT_name  ("F")
F<> f;
```
Using template parameters to reconstruct the name, without the default values, 
would result in the same names as the typedefs above create. That said,  it 
does feel slightly "incomplete" to leave them off. I'll have a look today to 
see whether its possible to do anything about that.

I'll add a test for default args either way, and move the code out into a 
function.


https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const 
TemplateSpecializationType *Ty,
   auto PP = getPrintingPolicy();
   Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
 
+  SourceLocation Loc = AliasDecl->getLocation();
+
+  if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+  ArrayRef SubstArgs = Ty->template_arguments();
+  for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+  SpecArgs.push_back(TemplateArgument(SubstArgs));
+  break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+  // If SubstArgs is now empty (we're taking from it each iteration) 
and
+  // this template parameter isn't a pack, then that should mean we're
+  // using default values for the remaining template parameters.
+  break;

OCHyams wrote:

I'm glad you pointed it out - it was doing what I wanted it to but I hadn't 
tried very hard to get it to gather default argument values, and I agree 
copying the class template instantiation debug info is the safest bet.

I've implemented that now. Slightly worried there are other template parameter 
kinds I've not thought of, currently guarded by an llvm_unrecahable. Maybe 
there's something better to do there - do you have any thoughts on this?

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -triple x86_64-unk-unk -o - -emit-llvm 
-debug-info-kind=standalone -gtemplate-alias %s -gsimple-template-names=simple \
+// RUN: | FileCheck %s
+
+ Check that -gtemplate-alias causes DW_TAG_template_alias emission for
+ template aliases with default parameter values. See template-alias.cpp for
+  more template alias tests.
+
+template
+struct X {
+  char m;
+};
+
+template
+struct Y {
+  char n;
+};
+
+template  class T = Y, int I = 5, 
typename... Ts>
+using A = X;
+
+ We should be able to emit type alias metadata which describes all the
+ values, including the defaulted parameters and empty parameter pack.
+A a;
+
+// CHECK: !DIDerivedType(tag: DW_TAG_template_alias, name: "A", file: ![[#]], 
line: [[#]], baseType: ![[baseType:[0-9]+]], extraData: ![[extraData:[0-9]+]])
+// CHECK: ![[baseType]] = distinct !DICompositeType(tag: 
DW_TAG_structure_type, name: "X",
+// CHECK: ![[int:[0-9]+]] = !DIBasicType(name: "int", size: 32, encoding: 
DW_ATE_signed)
+// CHECK: ![[extraData]] = !{![[NonDefault:[0-9]+]], ![[T:[0-9]+]], 
![[I:[0-9]+]], ![[Ts:[0-9]+]]}
+// CHECK: ![[NonDefault]] = !DITemplateTypeParameter(name: "NonDefault", type: 
![[int]])
+// CHECK: ![[T]] = !DITemplateValueParameter(tag: 
DW_TAG_GNU_template_template_param, name: "T", defaulted: true, value: !"Y")
+// CHECK: ![[I]] = !DITemplateValueParameter(name: "I", type: ![[int]], 
defaulted: true, value: i32 5)
+// CHECK: ![[Ts]] = !DITemplateValueParameter(tag: 
DW_TAG_GNU_template_parameter_pack, name: "Ts", value: ![[types:[0-9]+]])
+// CHECK: ![[types]] = !{}

OCHyams wrote:

For anyone interested, the dwarf for this test case looks like this:
```
$ clang -gsimple-template-names test9.cpp -g -c -o - -gtemplate-alias | 
llvm-dwarfdump - --name A --show-children
-:  file format elf64-x86-64

0x0029: DW_TAG_template_alias
  DW_AT_type(0x0044 "X")
  DW_AT_name("A")
  DW_AT_decl_file   ("/home/och/scratch/test9.cpp")
  DW_AT_decl_line   (13)

0x0031:   DW_TAG_template_type_parameter
DW_AT_type  (0x005a "int")
DW_AT_name  ("NonDefault")

0x0037:   DW_TAG_GNU_template_template_param
DW_AT_name  ("T")
DW_AT_default_value (true)
DW_AT_GNU_template_name ("Y")

0x003a:   DW_TAG_template_value_parameter
DW_AT_type  (0x005a "int")
DW_AT_name  ("I")
DW_AT_default_value (true)
DW_AT_const_value   (5)

0x0041:   DW_TAG_GNU_template_parameter_pack
DW_AT_name  ("Ts")

0x0043:   NULL
```

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const 
TemplateSpecializationType *Ty,
   auto PP = getPrintingPolicy();
   Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
 
+  SourceLocation Loc = AliasDecl->getLocation();
+
+  if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+  ArrayRef SubstArgs = Ty->template_arguments();
+  for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+  SpecArgs.push_back(TemplateArgument(SubstArgs));
+  break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+  // If SubstArgs is now empty (we're taking from it each iteration) 
and
+  // this template parameter isn't a pack, then that should mean we're
+  // using default values for the remaining template parameters.
+  break;

OCHyams wrote:

Ah - this implementation appears to cause an assertion when one of the template 
parameters is a dependant type... (working on it)

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const 
TemplateSpecializationType *Ty,
   auto PP = getPrintingPolicy();
   Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
 
+  SourceLocation Loc = AliasDecl->getLocation();
+
+  if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+  ArrayRef SubstArgs = Ty->template_arguments();
+  for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+  SpecArgs.push_back(TemplateArgument(SubstArgs));
+  break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+  // If SubstArgs is now empty (we're taking from it each iteration) 
and
+  // this template parameter isn't a pack, then that should mean we're
+  // using default values for the remaining template parameters.
+  break;

OCHyams wrote:

> The three kinds you check for should be sufficient afaik. Places around clang 
> consistently only deal with these three types of parameter decls.

Excellent, that is reassuring, thanks.

I'm wondering if you've got any ideas about my dependent value situation:

```
template 
using B = int;
B<> b;
```

The current default-argument-gathering tactic can't cope with dependent default 
expressions/types. Do you know if there's any way that I can "resolve", for 
lack of a better word, the template parameter default expression to get an 
expression `= 5` rather than the dependent `= I1`? Presumably this is doable, 
because we've got a specialisation that defines what `I1` is (are there cases 
where this could never work... maybe with SFINAE? I'm not sure).

I'm continuing to look into it, I'm only asking in case you know off the top of 
your head, no problem if not.
 

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const 
TemplateSpecializationType *Ty,
   auto PP = getPrintingPolicy();
   Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
 
+  SourceLocation Loc = AliasDecl->getLocation();
+
+  if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+  ArrayRef SubstArgs = Ty->template_arguments();
+  for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+  SpecArgs.push_back(TemplateArgument(SubstArgs));
+  break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+  // If SubstArgs is now empty (we're taking from it each iteration) 
and
+  // this template parameter isn't a pack, then that should mean we're
+  // using default values for the remaining template parameters.
+  break;

OCHyams wrote:

Looking at a class template instantiation with a default dependent value:
```
template 
struct B {};
B<> b;
```
We get:
```
0x0029:   DW_TAG_structure_type
DW_AT_calling_convention(DW_CC_pass_by_value)
DW_AT_name  ("B<5, 5>")
DW_AT_byte_size (0x01)
DW_AT_decl_file ("/home/och/scratch/test11.cpp")
DW_AT_decl_line (2)

0x002f: DW_TAG_template_value_parameter
  DW_AT_type(0x003e "int")
  DW_AT_name("I1")
  DW_AT_default_value   (true)
  DW_AT_const_value (5)

0x0036: DW_TAG_template_value_parameter
  DW_AT_type(0x003e "int")
  DW_AT_name("I2")
  DW_AT_default_value   (true)
  DW_AT_const_value (5)

0x003d: NULL
 ```
 
Is your suggestion to emit something like this for the template alias case 
(from my previous comment)?
```
0x0029:   DW_TAG_template_alias
DW_AT_name  ("B<5, ?>")
...

0x002f: DW_TAG_template_value_parameter
  DW_AT_type(0x003e "int")
  DW_AT_name("I1")
  DW_AT_default_value   (true)
  DW_AT_const_value (5)

0x0036: DW_TAG_template_value_parameter
  DW_AT_type(0x003e "int")
  DW_AT_name("I2")

0x003d: NULL
```

I think that is doable but I'm not sure how that would interact with name 
reconstruction (in Clang debug info without -gsimple-template-names, and in 
debuggers/tools with it).

tyvm for your ongoing help with this.

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const 
TemplateSpecializationType *Ty,
   auto PP = getPrintingPolicy();
   Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
 
+  SourceLocation Loc = AliasDecl->getLocation();
+
+  if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+  ArrayRef SubstArgs = Ty->template_arguments();
+  for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+  SpecArgs.push_back(TemplateArgument(SubstArgs));
+  break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+  // If SubstArgs is now empty (we're taking from it each iteration) 
and
+  // this template parameter isn't a pack, then that should mean we're
+  // using default values for the remaining template parameters.
+  break;

OCHyams wrote:

> Yup, would default values ever matter for name reconstruction?

I'd have thought not too, which is reflected in my initial approach of just 
leaving off defaulted args. However, it turns out our debugger displays 
defaulted values (this is intentional & desirable according to the team), and 
so does GDB.



https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const 
TemplateSpecializationType *Ty,
   auto PP = getPrintingPolicy();
   Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
 
+  SourceLocation Loc = AliasDecl->getLocation();
+
+  if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+  ArrayRef SubstArgs = Ty->template_arguments();
+  for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+  SpecArgs.push_back(TemplateArgument(SubstArgs));
+  break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+  // If SubstArgs is now empty (we're taking from it each iteration) 
and
+  // this template parameter isn't a pack, then that should mean we're
+  // using default values for the remaining template parameters.
+  break;

OCHyams wrote:

(defaulted values... and types*)

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const 
TemplateSpecializationType *Ty,
   auto PP = getPrintingPolicy();
   Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
 
+  SourceLocation Loc = AliasDecl->getLocation();
+
+  if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+  ArrayRef SubstArgs = Ty->template_arguments();
+  for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+  SpecArgs.push_back(TemplateArgument(SubstArgs));
+  break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+  // If SubstArgs is now empty (we're taking from it each iteration) 
and
+  // this template parameter isn't a pack, then that should mean we're
+  // using default values for the remaining template parameters.
+  break;

OCHyams wrote:

> I suppose you could call EvaluateKnownConstInt (on the TemplateArgument 
> expression, if it's !isValueDependent() && isIntegerConstantExpr(), similar 
> to what templateArgumentExpressionsEqual does). Maybe we can do this from 
> within CollectTemplateParams where it asserts that the expression is a 
> Constant?

The constant value default appears to be working, it's the dependent value `int 
I2 = I1` that is causing problems for me.

> Seems like you're tripping over the assert now because you're calling 
> CollectTemplateParams on the arguments to a TemplateAliasDecl which don't 
> have their arguments evaluated in the front-end it looks like.

That sounds like it might be the problem. It feels like we have all the parts 
needed to instantiate the dependent expression at this point... (because all 
the types/values it depends on exist as non-default argument values), I'm just 
not sure how to ask Clang to do that.


Here's an example using type parameters rather than values (you can also do 
`typename C = X` if you want to hit yet another assertion...)
```
template 
using A = int;
A<> a;
```

```
Dependent types cannot show up in debug information
UNREACHABLE executed at clang/lib/CodeGen/CGDebugInfo.cpp:3686!
...
CGDebugInfo::CreateTypeNode(clang::QualType, llvm::DIFile*)
CGDebugInfo::getOrCreateType(clang::QualType, llvm::DIFile*) 
CollectTemplateParams # from the TemplateArgument::Type case processing the 
type of the argument for C
```

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const 
TemplateSpecializationType *Ty,
   auto PP = getPrintingPolicy();
   Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
 
+  SourceLocation Loc = AliasDecl->getLocation();
+
+  if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+  ArrayRef SubstArgs = Ty->template_arguments();
+  for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+  SpecArgs.push_back(TemplateArgument(SubstArgs));
+  break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+  // If SubstArgs is now empty (we're taking from it each iteration) 
and
+  // this template parameter isn't a pack, then that should mean we're
+  // using default values for the remaining template parameters.
+  break;

OCHyams wrote:

>Sorry for diverting you down this path, maybe ignoring the default arguments 
>as you did in your first attempt is sufficient if this is just not feasible.

Not at all, I think it was worth exploring and I very much appreciate the 
detailed review and the help that's come with it.

I agree that ignoring default arguments might be the way forward then. It isn't 
a regression in terms of name reconstruction, since the existing typedef 
approach for template aliases omits defaulted arguments from the DW_AT_name 
anyway:

```
template 
using A = int;
A<> a;
```
`$ clang test.cpp -g -o - | llvm-dwarfdump -o -`
```
[...]
0x002e:   DW_TAG_typedef
DW_AT_type  (0x0036 "int")
DW_AT_name  ("A<>")  <-- no defaulted arguments for 
template aliases :-(
DW_AT_decl_file ("test.cpp")
DW_AT_decl_line (41)
```

I have pushed that change (to omit defaulted arguments). Thanks for bearing 
with me on this one!

I've chatted to our debugger folks and they're ok with omitting defaulted args 
(though ideally this would be fixed in the future).

I think moving this discussion onto discourse might be useful - I'll look at 
doing that later in the week or next week.

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

OCHyams wrote:

I've rebased this so now this pull request contains only the Clang changes 
(LLVM side is here #88943).

There's a discussion starting 
[here](https://github.com/llvm/llvm-project/pull/87623#discussion_r1567968127) 
that discusses whether or not we should include defaulted arguments in the 
list. The very short summary is that I tried this by scraping the defaults from 
the template parameter list, but it turns out this is difficult in the face of 
parameter defaults that are dependent types and values. So the current 
implementation ignores defaulted arguments, i.e., doesn't include them in the 
argument list (and as a consequence also omits empty parameter packs that come 
after defaulted arguments).

This is not ideal, but not a regression from the DW_TAG_typedef names which 
also do not include defaulted arg values.

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple x86_64-unk-unk -o - -emit-llvm 
-debug-info-kind=standalone -gtemplate-alias %s -gsimple-template-names=simple \
+// RUN: | FileCheck %s
+
+ Check that -gtemplate-alias causes DW_TAG_template_alias emission for
+ template aliases with default parameter values. See template-alias.cpp for
+  more template alias tests.
+ FIXME: We currently do not emit defaulted arguments.
+
+template
+struct X {
+  char m;
+};
+
+template
+struct Y {
+  char n;
+};
+
+template  class T = Y, int I = 5, 
typename... Ts>
+using A = X;
+
+ We should be able to emit type alias metadata which describes all the
+ values, including the defaulted parameters and empty parameter pack.
+A a;
+
+// CHECK: !DIDerivedType(tag: DW_TAG_template_alias, name: "A", file: ![[#]], 
line: [[#]], baseType: ![[baseType:[0-9]+]], extraData: ![[extraData:[0-9]+]])
+// CHECK: ![[baseType]] = distinct !DICompositeType(tag: 
DW_TAG_structure_type, name: "X",
+// CHECK: ![[int:[0-9]+]] = !DIBasicType(name: "int", size: 32, encoding: 
DW_ATE_signed)
+// CHECK: ![[extraData]] = !{![[NonDefault:[0-9]+]]}
+// CHECK: ![[NonDefault]] = !DITemplateTypeParameter(name: "NonDefault", type: 
![[int]])
+
+ FIXME: Ideally, we would describe the deafulted args, like this:
+// : ![[extraData]] = !{![[NonDefault:[0-9]+]], ![[T:[0-9]+]], ![[I:[0-9]+]], 
![[Ts:[0-9]+]]}

OCHyams wrote:

I think the test will already start failing if this gets fixed because the 
current `extraData` metadata tuple contents check will fail (it'll have more 
than one element). I'm still happy to add CHECK-NOTs if you'd like, just 
thought I'd raise this in case it changes your stance.

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

OCHyams wrote:

Thanks everyone. I'll land this now then to see what the bots have to say about 
it.

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams closed 
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix template alias default DWARF version (PR #89594)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams created 
https://github.com/llvm/llvm-project/pull/89594

DW_TAG_template_alias DIEs were added in DWARFv4, not DWARFv5

>From 95c86b499e1eebb15dbd61f839f5fefa83dc910d Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams 
Date: Mon, 22 Apr 2024 11:48:19 +0100
Subject: [PATCH 1/2] [Clang] Fix template alias default DWARF versiion

DW_TAG_template_alias DIEs were added in DWARFv4, not DWARFv5
---
 clang/lib/Driver/ToolChains/Clang.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 97b4aa1c9b1d0a..f8a81ee8ab56bc 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4634,7 +4634,7 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, 
const llvm::Triple &T,
 
   // Emit DW_TAG_template_alias for template aliases? True by default for SCE.
   bool UseDebugTemplateAlias =
-  DebuggerTuning == llvm::DebuggerKind::SCE && RequestedDWARFVersion >= 5;
+  DebuggerTuning == llvm::DebuggerKind::SCE && RequestedDWARFVersion >= 4;
   if (const auto *DebugTemplateAlias = Args.getLastArg(
   options::OPT_gtemplate_alias, options::OPT_gno_template_alias)) {
 // DW_TAG_template_alias is only supported from DWARFv5 but if a user

>From 90235c8e9d953975479f18a2cb8aa05a3d3456b7 Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams 
Date: Mon, 22 Apr 2024 11:55:32 +0100
Subject: [PATCH 2/2] update test

---
 clang/test/Driver/debug-options.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/test/Driver/debug-options.c 
b/clang/test/Driver/debug-options.c
index b209c911d1ca2b..7d061410a229f0 100644
--- a/clang/test/Driver/debug-options.c
+++ b/clang/test/Driver/debug-options.c
@@ -456,9 +456,9 @@
 // RUN: %clang -### -target x86_64 -c -g %s 2>&1 | FileCheck 
--check-prefix=FULL_TEMP_NAMES 
--implicit-check-not=debug-forward-template-params %s
 // FULL_TEMP_NAMES-NOT: -gsimple-template-names
 
- Test -g[no-]template-alias (enabled by default with SCE debugger tuning 
and DWARFv5).
+ Test -g[no-]template-alias (enabled by default with SCE debugger tuning 
and DWARF version >= 4).
 // RUN: %clang -### -target x86_64 -c -gdwarf-5 -gsce %s 2>&1 | FileCheck %s 
--check-prefixes=TEMPLATE-ALIAS
-// RUN: %clang -### -target x86_64 -c -gdwarf-4 -gsce %s 2>&1 | FileCheck %s 
--check-prefixes=NO-TEMPLATE-ALIAS
+// RUN: %clang -### -target x86_64 -c -gdwarf-3 -gsce %s 2>&1 | FileCheck %s 
--check-prefixes=NO-TEMPLATE-ALIAS
 // RUN: %clang -### -target x86_64 -c -gdwarf-5 -gsce -gtemplate-alias %s 2>&1 
| FileCheck %s --check-prefixes=TEMPLATE-ALIAS
 // RUN: %clang -### -target x86_64 -c -gdwarf-5 -gsce -gno-template-alias %s 
2>&1 | FileCheck %s --check-prefixes=NO-TEMPLATE-ALIAS
 // RUN: %clang -### -target x86_64 -c -gdwarf-5 -gtemplate-alias %s 2>&1 | 
FileCheck %s --check-prefixes=TEMPLATE-ALIAS

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix template alias default DWARF version (PR #89594)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams closed 
https://github.com/llvm/llvm-project/pull/89594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

OCHyams wrote:

Dependent expressions strike again - https://godbolt.org/z/W381837vr
```
template 
using A = int;

template
struct S {
  using AA = A;
  AA aa;
};

S<0> s;
```
` clang++ -c test.cpp -g -gtemplate-alias`
`clang/lib/AST/ExprConstant.cpp:15721: bool 
clang::Expr::EvaluateAsRValue(clang::Expr::EvalResult&, const 
clang::ASTContext&, bool) const: Assertion '!isValueDependent() && "Expression 
evaluator can't be called on a dependent expression."' failed.`

The issue is `I` is a dependent expression. We have an actual instantiation of 
S with the value of `I` clearly being `0` here, but I'm not sure whether that 
information exists in the AST for the `TemplateSpecializationType *`.

Looking into this now (will revert if I can't find a solution).

https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fall back to DW_TAG_typedef for instantiation dependent template aliases (PR #90032)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams created 
https://github.com/llvm/llvm-project/pull/90032

Workaround for issue #89774 until it can be properly fixed.

When `-gtemplate-alias` is specified Clang emits a DW_TAG_template_alias for 
template aliases. This patch avoids an assertion failure by falling back to the 
 `-gno-template-alias` (default) behaviour, emitting a DW_TAG_typedef, if the 
alias is instantiation dependent.



>From 075a3f662807d2605964bd20b17e9552c07098be Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams 
Date: Thu, 25 Apr 2024 09:30:05 +0100
Subject: [PATCH] [Clang] Fall back to DW_TAG_typedef for instantiation
 dependent template aliases

Workaround for issue #89774
---
 clang/lib/CodeGen/CGDebugInfo.cpp | 21 ++-
 .../CodeGenCXX/dependent-template-alias.cpp   | 21 +++
 2 files changed, 41 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGenCXX/dependent-template-alias.cpp

diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 539ded5cca5e1b..787db350487417 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1372,7 +1372,26 @@ llvm::DIType *CGDebugInfo::CreateType(const 
TemplateSpecializationType *Ty,
 
   SourceLocation Loc = AliasDecl->getLocation();
 
-  if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+  if (CGM.getCodeGenOpts().DebugTemplateAlias &&
+  // The TemplateSpecializationType doesn't contain any instantiation
+  // information; dependent template arguments can't be resolved. For now,
+  // fall back to DW_TAG_typedefs for template aliases that are
+  // instantiation dependent, e.g.:
+  // ```
+  // template 
+  // using A = int;
+  //
+  // template
+  // struct S {
+  //   using AA = A; // Instantiation dependent.
+  //   AA aa;
+  // };
+  //
+  // S<0> s;
+  // ```
+  // S::AA's underlying type A is dependent on I so will be emitted as a
+  // DW_TAG_typedef.
+  !Ty->isInstantiationDependentType()) {
 auto ArgVector = ::GetTemplateArgs(TD, Ty);
 TemplateArgs Args = {TD->getTemplateParameters(), ArgVector};
 
diff --git a/clang/test/CodeGenCXX/dependent-template-alias.cpp 
b/clang/test/CodeGenCXX/dependent-template-alias.cpp
new file mode 100644
index 00..deb243f9fc88d0
--- /dev/null
+++ b/clang/test/CodeGenCXX/dependent-template-alias.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple x86_64-unk-unk -o - -emit-llvm 
-debug-info-kind=standalone -gtemplate-alias %s -gsimple-template-names=simple \
+// RUN: | FileCheck %s
+
+ Check that -gtemplate-alias falls back to DW_TAG_typedef emission
+ for instantiation dependent type aliases.
+
+template 
+using A = int;
+
+template
+struct S {
+  using AA = A;
+  AA aa;
+};
+
+S<0> s;
+
+// CHECK: !DIDerivedType(tag: DW_TAG_member, name: "aa", scope: ![[#]], file: 
![[#]], line: [[#]], baseType: ![[AA:[0-9]+]], size: 32)
+// CHECK: [[AA]] = !DIDerivedType(tag: DW_TAG_typedef, name: "AA", file: 
![[#]], line: [[#]], baseType: ![[A:[0-9]+]])
+// CHECK: [[A]] = !DIDerivedType(tag: DW_TAG_typedef, name: "A", file: 
![[#]], line: [[#]], baseType: ![[int:[0-9]+]])
+// CHECK: [[int]] = !DIBasicType(name: "int", size: 32, encoding: 
DW_ATE_signed)

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fall back to DW_TAG_typedef for instantiation dependent template aliases (PR #90032)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams closed 
https://github.com/llvm/llvm-project/pull/90032
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] f78949a - [NFC][Clang] Add FIXME comment to the workaround for issue #89774

[Orlando Cazalet-Hyams via cfe-commits]

Author: Orlando Cazalet-Hyams
Date: 2024-04-30T09:16:14+01:00
New Revision: f78949a07e33017a798c410a102c95455685a9b1

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

LOG: [NFC][Clang] Add FIXME comment to the workaround for issue #89774

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 787db350487417..fac278f0e20a43 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1373,6 +1373,8 @@ llvm::DIType *CGDebugInfo::CreateType(const 
TemplateSpecializationType *Ty,
   SourceLocation Loc = AliasDecl->getLocation();
 
   if (CGM.getCodeGenOpts().DebugTemplateAlias &&
+  // FIXME: This is a workaround for the issue
+  //https://github.com/llvm/llvm-project/issues/89774
   // The TemplateSpecializationType doesn't contain any instantiation
   // information; dependent template arguments can't be resolved. For now,
   // fall back to DW_TAG_typedefs for template aliases that are



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fall back to DW_TAG_typedef for instantiation dependent template aliases (PR #90032)

[Orlando Cazalet-Hyams via cfe-commits]

OCHyams wrote:

> Comment in the code should probably mention this as a FIXME and include a 
> reference to the issue?

Sure, added in f78949a07e33017a798c410a102c95455685a9b1

> Also, there's another bug here - the DW_TAG_typedef is in the CU scope, 
> instead of the struct scope. But if the struct is a non-template, the typedef 
> is in the struct scope as it should be, not the CU scope...

That does looks odd - I can reproduce it with normal (language-level) typedefs 
too (rather than with template aliases): https://godbolt.org/z/GsGKqhKzz. I can 
open a separate issue?

Now that I think about it, I recall @CarlosAlbertoEnciso running into something 
similar a while ago... I'm sure a bug was filed about something similar but I 
can't find it. Does this ring any bells @CarlosAlbertoEnciso?


https://github.com/llvm/llvm-project/pull/90032
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)

[Orlando Cazalet-Hyams via cfe-commits]

OCHyams wrote:

@Michael137 said:


> Btw, as a follow-up to this patch should we check that this is compatible 
> with dsymutil (i.e., running dsymutil --verify)? I suspect it might need a 
> fixup (given LLDB doesn't even support this tag)

`dsymutil --verify` seems to be happy with the tag. Upon closer inspection, I 
think it uses the same "verifier" code as llvm-dwarfdump so that makes sense 
(#89589).

There's a couple of switches in llvm/lib/DWARFLinker that look like they want a 
DW_TAG_template_alias:
`DependencyTracker::isTypeTableCandidate` in `DependencyTracker.cpp` 
`AcceleratorRecordsSaver::save` in `AcceleratorRecordsSaver.cpp`. I'm not sure 
what a test for those would look like but I can look into it if you'd like?



https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fall back to DW_TAG_typedef for instantiation dependent template aliases (PR #90032)

[Orlando Cazalet-Hyams via cfe-commits]

OCHyams wrote:

Here's that issue - #91451.

Carlos pointed me to [bz44884](https://bugs.llvm.org/show_bug.cgi?id=44884) 
(#44229) which may be related

https://github.com/llvm/llvm-project/pull/90032
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [DebugInfo][RemoveDIs] Use iterator-inserters in clang (PR #102006)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams approved this pull request.

LGTM with nit + pls appease the the formatter

https://github.com/llvm/llvm-project/pull/102006
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [DebugInfo][RemoveDIs] Use iterator-inserters in clang (PR #102006)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams edited 
https://github.com/llvm/llvm-project/pull/102006
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [DebugInfo][RemoveDIs] Use iterator-inserters in clang (PR #102006)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -295,18 +295,25 @@ void EHScopeStack::Cleanup::anchor() {}
 static void createStoreInstBefore(llvm::Value *value, Address addr,
   llvm::Instruction *beforeInst,
   CodeGenFunction &CGF) {
-  auto store = new llvm::StoreInst(value, addr.emitRawPointer(CGF), 
beforeInst);
+  auto store = new llvm::StoreInst(value, addr.emitRawPointer(CGF),
+   beforeInst->getIterator());

OCHyams wrote:

Is it worth updating the `beforeInst` parameter to a iterator instead of 
`->getIterator()`-ing here? That would match the `createLoadInstBefore` 
overload you've added in this patch too.

https://github.com/llvm/llvm-project/pull/102006
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [DebugInfo] Update DIBuilder insertion to take InsertPosition (PR #126059)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams commented:

> @OCHyams I believe you did the C API changes, are there any additional 
> concerns in this area?

SGTM, just one inline question

https://github.com/llvm/llvm-project/pull/126059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [DebugInfo] Update DIBuilder insertion to take InsertPosition (PR #126059)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams edited 
https://github.com/llvm/llvm-project/pull/126059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [DebugInfo] Update DIBuilder insertion to take InsertPosition (PR #126059)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -1686,7 +1686,8 @@ LLVMDbgRecordRef LLVMDIBuilderInsertDeclareRecordBefore(
   DbgInstPtr DbgInst = unwrap(Builder)->insertDeclare(
   unwrap(Storage), unwrap(VarInfo),
   unwrap(Expr), unwrap(DL),
-  unwrap(Instr));
+  Instr ? InsertPosition(unwrap(Instr)->getIterator())
+: nullptr);

OCHyams wrote:

Wouldn't this trip an assertion in the `nullptr` InsertPosition case 
(`assert(InsertPt.isValid());` in `insertDbgVariableRecord` called by 
`insertDeclare`). Or am I reading this wrong? (Same question applies to the 
other calls below)

https://github.com/llvm/llvm-project/pull/126059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [polly] [NFC][DebugInfo] Use iterator moveBefore at many call-sites (PR #123583)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -242,13 +253,19 @@ class Instruction : public User,
   /// \pre I is a valid iterator into BB.
   void moveBefore(BasicBlock &BB, InstListType::iterator I);
 
-  /// (See other overload for moveBeforePreserving).
   void moveBeforePreserving(BasicBlock &BB, InstListType::iterator I);
+  /// Unlink this instruction from its current basic block and insert it into
+  /// the basic block that MovePos lives in, right before MovePos.

OCHyams wrote:

Nit: mismatch param name in comment

https://github.com/llvm/llvm-project/pull/123583
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [polly] [NFC][DebugInfo] Use iterator moveBefore at many call-sites (PR #123583)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -224,11 +231,15 @@ class Instruction : public User,
   /// the basic block that MovePos lives in, right before MovePos.
   void moveBefore(Instruction *MovePos);
 
+  /// Unlink this instruction from its current basic block and insert it into
+  /// the basic block that MovePos lives in, right before MovePos.
+  void moveBefore(InstListType::iterator InsertPos);

OCHyams wrote:

```suggestion
  /// the basic block that MovePos lives in, right before MovePos.
  void moveBefore(InstListType::iterator MovePos);
```

https://github.com/llvm/llvm-project/pull/123583
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [polly] [NFC][DebugInfo] Use iterator moveBefore at many call-sites (PR #123583)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -448,7 +448,7 @@ void IRPromoter::ExtendSources() {
   if (isa(V))
 I->moveBefore(InsertPt);
   else
-I->moveAfter(InsertPt);
+I->moveAfter(&*InsertPt);

OCHyams wrote:

Same as earlier - is this a code-transition-state bug, or is there some reason 
we've got to use the `Instruction *` overload here? (this one matters more)

https://github.com/llvm/llvm-project/pull/123583
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [polly] [NFC][DebugInfo] Use iterator moveBefore at many call-sites (PR #123583)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams approved this pull request.

LGTM with a couple nit/questions.

https://github.com/llvm/llvm-project/pull/123583
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [polly] [NFC][DebugInfo] Use iterator moveBefore at many call-sites (PR #123583)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -8890,21 +8890,21 @@ bool 
CodeGenPrepare::fixupDbgVariableRecord(DbgVariableRecord &DVR) {
   return AnyChange;
 }
 
-static void DbgInserterHelper(DbgValueInst *DVI, Instruction *VI) {
+static void DbgInserterHelper(DbgValueInst *DVI, BasicBlock::iterator VI) {
   DVI->removeFromParent();
   if (isa(VI))
-DVI->insertBefore(&*VI->getParent()->getFirstInsertionPt());
+DVI->insertBefore(VI->getParent()->getFirstInsertionPt());
   else
 DVI->insertAfter(VI);
 }
 
-static void DbgInserterHelper(DbgVariableRecord *DVR, Instruction *VI) {
+static void DbgInserterHelper(DbgVariableRecord *DVR, BasicBlock::iterator VI) 
{
   DVR->removeFromParent();
   BasicBlock *VIBB = VI->getParent();
   if (isa(VI))
 VIBB->insertDbgRecordBefore(DVR, VIBB->getFirstInsertionPt());
   else
-VIBB->insertDbgRecordAfter(DVR, VI);
+VIBB->insertDbgRecordAfter(DVR, &*VI);

OCHyams wrote:

Is this a code-transition-state bug, or is there some reason we've got to use 
the Instruction * overload here? I suppose it doesn't matter either way for dbg 
records.

https://github.com/llvm/llvm-project/pull/123583
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [polly] [NFC][DebugInfo] Use iterator moveBefore at many call-sites (PR #123583)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams edited 
https://github.com/llvm/llvm-project/pull/123583
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [polly] [NFC][DebugInfo] Use iterator moveBefore at many call-sites (PR #123583)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -224,11 +231,15 @@ class Instruction : public User,
   /// the basic block that MovePos lives in, right before MovePos.
   void moveBefore(Instruction *MovePos);
 
+  /// Unlink this instruction from its current basic block and insert it into
+  /// the basic block that MovePos lives in, right before MovePos.
+  void moveBefore(InstListType::iterator InsertPos);

OCHyams wrote:

nit: parameter name and comment don't match up

https://github.com/llvm/llvm-project/pull/123583
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [polly] [NFC][DebugInfo] Use iterator moveBefore at many call-sites (PR #123583)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -224,11 +231,15 @@ class Instruction : public User,
   /// the basic block that MovePos lives in, right before MovePos.
   void moveBefore(Instruction *MovePos);
 
+  /// Unlink this instruction from its current basic block and insert it into
+  /// the basic block that MovePos lives in, right before MovePos.
+  void moveBefore(InstListType::iterator InsertPos);
+
   /// Perform a \ref moveBefore operation, while signalling that the caller
   /// intends to preserve the original ordering of instructions. This 
implicitly
   /// means that any adjacent debug-info should move with this instruction.
-  /// This method is currently a no-op placeholder, but it will become 
meaningful
-  /// when the "RemoveDIs" project is enabled.
+  /// This method is currently a no-op placeholder, but it will become
+  /// meaningful when the "RemoveDIs" project is enabled.

OCHyams wrote:

Is this comment stale?

https://github.com/llvm/llvm-project/pull/123583
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [NFC][DebugInfo] Rewrite more call-sites to insert with iterators (PR #124288)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams edited 
https://github.com/llvm/llvm-project/pull/124288
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [NFC][DebugInfo] Rewrite more call-sites to insert with iterators (PR #124288)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -823,7 +823,16 @@ static void updateScopeLine(Instruction *ActiveSuspend,
   if (!ActiveSuspend)
 return;
 
-  auto *Successor = ActiveSuspend->getNextNonDebugInstruction();
+  // No subsequent instruction -> fallback to the location of ActiveSuspend.
+  if (!ActiveSuspend->getNextNonDebugInstruction()) {
+if (auto DL = ActiveSuspend->getDebugLoc())
+  if (SPToUpdate.getFile() == DL->getFile())
+SPToUpdate.setScopeLine(DL->getLine());
+return;
+  }
+
+  BasicBlock::iterator Successor =

OCHyams wrote:

this doesn't break the `dyn_cast_or_null` below?

https://github.com/llvm/llvm-project/pull/124288
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [NFC][DebugInfo] Rewrite more call-sites to insert with iterators (PR #124288)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams approved this pull request.

> parent-pointers can now be accessed from ilist nodes

Feels a bit of a shame we're not using this right away, but LGTM + nits

https://github.com/llvm/llvm-project/pull/124288
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [NFC][DebugInfo] Use iterators for instruction insertion in more places (PR #124291)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/124291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [DebugInfo] Update DIBuilder insertion to take InsertPosition (PR #126059)

[Orlando Cazalet-Hyams via cfe-commits]

@@ -1686,7 +1686,8 @@ LLVMDbgRecordRef LLVMDIBuilderInsertDeclareRecordBefore(
   DbgInstPtr DbgInst = unwrap(Builder)->insertDeclare(
   unwrap(Storage), unwrap(VarInfo),
   unwrap(Expr), unwrap(DL),
-  unwrap(Instr));
+  Instr ? InsertPosition(unwrap(Instr)->getIterator())
+: nullptr);

OCHyams wrote:

Thanks for unpicking that.

> This is a messy situation and in this PR I tried to just keep that exactly 
> the same. What previously worked should still work. What previously didn't 
> work should still not work.

SGTM. I feel as though these functions should probably have asserts in them 
saying as such but that could reasonably be argued to be out of scope of the 
patch.

Patch LGTM to me too.

https://github.com/llvm/llvm-project/pull/126059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Assignment Tracking] Replace `undef` debug info with `poison` (PR #129755)

[Orlando Cazalet-Hyams via cfe-commits]

https://github.com/OCHyams approved this pull request.

LGTM, thanks

https://github.com/llvm/llvm-project/pull/129755
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 3086546 - [Clang][NFC] Move some static functions into CodeGenFunction (#134634)

[Orlando Cazalet-Hyams via cfe-commits]

Author: Orlando Cazalet-Hyams
Date: 2025-04-08T08:44:10+01:00
New Revision: 308654608cb8bc5bbd5d4b3779cb7d92920dd6b7

URL: 
https://github.com/llvm/llvm-project/commit/308654608cb8bc5bbd5d4b3779cb7d92920dd6b7
DIFF: 
https://github.com/llvm/llvm-project/commit/308654608cb8bc5bbd5d4b3779cb7d92920dd6b7.diff

LOG: [Clang][NFC] Move some static functions into CodeGenFunction (#134634)

Patches in the Key Instructions (KeyInstr) stack need to access CGF in these
functions. 2 CGF fields are passed to these functions already; at this point it
felt natural to promote them to CGF methods.

Added: 


Modified: 
clang/lib/CodeGen/CGDecl.cpp
clang/lib/CodeGen/CodeGenFunction.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index eab1ebfb2369b..0af170a36f372 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -928,10 +928,9 @@ static bool 
canEmitInitWithFewStoresAfterBZero(llvm::Constant *Init,
 
 /// For inits that canEmitInitWithFewStoresAfterBZero returned true for, emit
 /// the scalar stores that would be required.
-static void emitStoresForInitAfterBZero(CodeGenModule &CGM,
-llvm::Constant *Init, Address Loc,
-bool isVolatile, CGBuilderTy &Builder,
-bool IsAutoInit) {
+void CodeGenFunction::emitStoresForInitAfterBZero(llvm::Constant *Init,
+  Address Loc, bool isVolatile,
+  bool IsAutoInit) {
   assert(!Init->isNullValue() && !isa(Init) &&
  "called emitStoresForInitAfterBZero for zero or undef value.");
 
@@ -952,8 +951,8 @@ static void emitStoresForInitAfterBZero(CodeGenModule &CGM,
   // If necessary, get a pointer to the element and emit it.
   if (!Elt->isNullValue() && !isa(Elt))
 emitStoresForInitAfterBZero(
-CGM, Elt, Builder.CreateConstInBoundsGEP2_32(Loc, 0, i), 
isVolatile,
-Builder, IsAutoInit);
+Elt, Builder.CreateConstInBoundsGEP2_32(Loc, 0, i), isVolatile,
+IsAutoInit);
 }
 return;
   }
@@ -966,9 +965,9 @@ static void emitStoresForInitAfterBZero(CodeGenModule &CGM,
 
 // If necessary, get a pointer to the element and emit it.
 if (!Elt->isNullValue() && !isa(Elt))
-  emitStoresForInitAfterBZero(CGM, Elt,
+  emitStoresForInitAfterBZero(Elt,
   Builder.CreateConstInBoundsGEP2_32(Loc, 0, 
i),
-  isVolatile, Builder, IsAutoInit);
+  isVolatile, IsAutoInit);
   }
 }
 
@@ -1169,10 +1168,10 @@ static Address 
createUnnamedGlobalForMemcpyFrom(CodeGenModule &CGM,
   return SrcPtr.withElementType(CGM.Int8Ty);
 }
 
-static void emitStoresForConstant(CodeGenModule &CGM, const VarDecl &D,
-  Address Loc, bool isVolatile,
-  CGBuilderTy &Builder,
-  llvm::Constant *constant, bool IsAutoInit) {
+void CodeGenFunction::emitStoresForConstant(const VarDecl &D, Address Loc,
+bool isVolatile,
+llvm::Constant *constant,
+bool IsAutoInit) {
   auto *Ty = constant->getType();
   uint64_t ConstantSize = CGM.getDataLayout().getTypeAllocSize(Ty);
   if (!ConstantSize)
@@ -1201,8 +1200,7 @@ static void emitStoresForConstant(CodeGenModule &CGM, 
const VarDecl &D,
 constant->isNullValue() || isa(constant);
 if (!valueAlreadyCorrect) {
   Loc = Loc.withElementType(Ty);
-  emitStoresForInitAfterBZero(CGM, constant, Loc, isVolatile, Builder,
-  IsAutoInit);
+  emitStoresForInitAfterBZero(constant, Loc, isVolatile, IsAutoInit);
 }
 return;
   }
@@ -1240,7 +1238,7 @@ static void emitStoresForConstant(CodeGenModule &CGM, 
const VarDecl &D,
   CharUnits::fromQuantity(Layout->getElementOffset(i));
   Address EltPtr = Builder.CreateConstInBoundsByteGEP(
   Loc.withElementType(CGM.Int8Ty), CurOff);
-  emitStoresForConstant(CGM, D, EltPtr, isVolatile, Builder,
+  emitStoresForConstant(D, EltPtr, isVolatile,
 constant->getAggregateElement(i), IsAutoInit);
 }
 return;
@@ -1251,7 +1249,7 @@ static void emitStoresForConstant(CodeGenModule &CGM, 
const VarDecl &D,
 for (unsigned i = 0; i != ATy->getNumElements(); i++) {
   Address EltPtr = Builder.CreateConstGEP(
   Loc.withElementType(ATy->getElementType()), i);
-  emitStoresForConstant(CGM, D, EltPtr, isVolatile, Builder,
+  emitStoresForConstant(D, EltPtr, isVolatile,
 c