[Lldb-commits] [lldb] 0ef58c6 - [LLDB][RISCV] Add RVDC instruction support for EmulateInstructionRISCV

[via lldb-commits]

Author: Emmmer
Date: 2023-01-13T20:52:34+08:00
New Revision: 0ef58c66c6e4652ff3582bf0972673708afb912e

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

LOG: [LLDB][RISCV] Add RVDC instruction support for EmulateInstructionRISCV

RVC is the RISC-V standard compressed instruction-set extension, named "C", 
which reduces static and dynamic code size by adding short 16-bit instruction 
encodings for common operations, and RVCD is the compressed "D extension".

And "D extension" is a double-precision floating-point instruction-set 
extension, which adds double-precision floating-point computational 
instructions compliant with the IEEE 754-2008 arithmetic standard.

Reviewed By: DavidSpickett

Differential Revision: https://reviews.llvm.org/D140961

Added: 


Modified: 
lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
lldb/source/Plugins/Instruction/RISCV/RISCVCInstructions.h
lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp 
b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
index 4fe025ad5d1af..06772aaa6e17c 100644
--- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
+++ b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
@@ -541,6 +541,11 @@ static const InstrPattern PATTERNS[] = {
 {"FSW", 0xE003, 0xE000, DecodeC_FSW, RV32},
 {"FLWSP", 0xE003, 0x6002, DecodeC_FLWSP, RV32},
 {"FSWSP", 0xE003, 0xE002, DecodeC_FSWSP, RV32},
+// RVDC //
+{"FLDSP", 0xE003, 0x2002, DecodeC_FLDSP, RV32 | RV64},
+{"FSDSP", 0xE003, 0xA002, DecodeC_FSDSP, RV32 | RV64},
+{"FLD", 0xE003, 0x2000, DecodeC_FLD, RV32 | RV64},
+{"FSD", 0xE003, 0xA000, DecodeC_FSD, RV32 | RV64},
 
 // RV32F (Extension for Single-Precision Floating-Point) //
 {"FLW", 0x707F, 0x2007, DecodeIType},

diff  --git a/lldb/source/Plugins/Instruction/RISCV/RISCVCInstructions.h 
b/lldb/source/Plugins/Instruction/RISCV/RISCVCInstructions.h
index 1f1d2853ab36d..867f067f24b38 100644
--- a/lldb/source/Plugins/Instruction/RISCV/RISCVCInstructions.h
+++ b/lldb/source/Plugins/Instruction/RISCV/RISCVCInstructions.h
@@ -327,5 +327,31 @@ RISCVInst DecodeC_FSWSP(uint32_t inst) {
   return FSW{Rs{gpr_sp_riscv}, DecodeCSS_RS2(inst), uint32_t(offset)};
 }
 
+RISCVInst DecodeC_FLDSP(uint32_t inst) {
+  auto rd = DecodeCI_RD(inst);
+  uint16_t offset = ((inst << 4) & 0x1c0)   // offset[8:6]
+| ((inst >> 7) & 0x20)  // offset[5]
+| ((inst >> 2) & 0x18); // offset[4:3]
+  return FLD{rd, Rs{gpr_sp_riscv}, uint32_t(offset)};
+}
+
+RISCVInst DecodeC_FSDSP(uint32_t inst) {
+  uint16_t offset = ((inst >> 1) & 0x1c0)   // offset[8:6]
+| ((inst >> 7) & 0x38); // offset[5:3]
+  return FSD{Rs{gpr_sp_riscv}, DecodeCSS_RS2(inst), uint32_t(offset)};
+}
+
+RISCVInst DecodeC_FLD(uint32_t inst) {
+  uint16_t offset = ((inst << 1) & 0xc0)// imm[7:6]
+| ((inst >> 7) & 0x38); // imm[5:3]
+  return FLD{DecodeCL_RD(inst), DecodeCL_RS1(inst), uint32_t(offset)};
+}
+
+RISCVInst DecodeC_FSD(uint32_t inst) {
+  uint16_t offset = ((inst << 1) & 0xc0)// imm[7:6]
+| ((inst >> 7) & 0x38); // imm[5:3]
+  return FSD{DecodeCS_RS1(inst), DecodeCS_RS2(inst), uint32_t(offset)};
+}
+
 } // namespace lldb_private
 #endif // LLDB_SOURCE_PLUGINS_INSTRUCTION_RISCV_RISCVCINSTRUCTION_H

diff  --git a/lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp 
b/lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp
index 406c0ae8f4b56..90d5a7c4f3b97 100644
--- a/lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp
+++ b/lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp
@@ -287,8 +287,10 @@ TEST_F(RISCVEmulatorTester, TestCDecode) {
   {0x0010, RESERVED{0x0010}},
   // ADDI4SPN here, decode as ADDI
   {0x0024, ADDI{Rd{9}, Rs{2}, 8}},
+  {0x2084, FLD{Rd{9}, Rs{9}, 0}},
   {0x4488, LW{Rd{10}, Rs{9}, 8}},
   {0x6488, LD{Rd{10}, Rs{9}, 8}},
+  {0xA084, FSD{Rs{9}, Rs{9}, 0}},
   {0xC488, SW{Rs{9}, Rs{10}, 8}},
   {0xE488, SD{Rs{9}, Rs{10}, 8}},
   {0x1001, NOP{0x1001}},
@@ -315,6 +317,8 @@ TEST_F(RISCVEmulatorTester, TestCDecode) {
   {0x1002, HINT{0x1002}},
   // SLLI64 here, decoded as HINT if not in RV128
   {0x0082, HINT{0x0082}},
+  // FLDSP here, decoded as FLD
+  {0x2082, FLD{Rd{1}, Rs{2}, 0}},
   // LWSP here, decoded as LW
   {0x4082, LW{Rd{1}, Rs{2}, 0}},
   // LDSP here, decoded as LD
@@ -326,6 +330,8 @@ TEST_F(RISCVEmulatorTester, TestCDecode) {
   {0x9002, EBREAK{0x9002}},
   {0x9082, JALR{Rd{1}, Rs{1}, 0}},
   {0x9086, ADD{Rd{1}, Rs{1}, Rs{1}}},
+  // C.FSDSP here, decoded as FSD
+

[Lldb-commits] [PATCH] D140961: [LLDB][RISCV] Add RVDC instruction support for EmulateInstructionRISCV

[Emmmer S via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Emmmer marked an inline comment as done.
Closed by commit rG0ef58c66c6e4: [LLDB][RISCV] Add RVDC instruction support for 
EmulateInstructionRISCV (authored by Emmmer).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140961/new/

https://reviews.llvm.org/D140961

Files:
  lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
  lldb/source/Plugins/Instruction/RISCV/RISCVCInstructions.h
  lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp


Index: lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp
===
--- lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp
+++ lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp
@@ -287,8 +287,10 @@
   {0x0010, RESERVED{0x0010}},
   // ADDI4SPN here, decode as ADDI
   {0x0024, ADDI{Rd{9}, Rs{2}, 8}},
+  {0x2084, FLD{Rd{9}, Rs{9}, 0}},
   {0x4488, LW{Rd{10}, Rs{9}, 8}},
   {0x6488, LD{Rd{10}, Rs{9}, 8}},
+  {0xA084, FSD{Rs{9}, Rs{9}, 0}},
   {0xC488, SW{Rs{9}, Rs{10}, 8}},
   {0xE488, SD{Rs{9}, Rs{10}, 8}},
   {0x1001, NOP{0x1001}},
@@ -315,6 +317,8 @@
   {0x1002, HINT{0x1002}},
   // SLLI64 here, decoded as HINT if not in RV128
   {0x0082, HINT{0x0082}},
+  // FLDSP here, decoded as FLD
+  {0x2082, FLD{Rd{1}, Rs{2}, 0}},
   // LWSP here, decoded as LW
   {0x4082, LW{Rd{1}, Rs{2}, 0}},
   // LDSP here, decoded as LD
@@ -326,6 +330,8 @@
   {0x9002, EBREAK{0x9002}},
   {0x9082, JALR{Rd{1}, Rs{1}, 0}},
   {0x9086, ADD{Rd{1}, Rs{1}, Rs{1}}},
+  // C.FSDSP here, decoded as FSD
+  {0xA006, FSD{Rs{2}, Rs{1}, 0}},
   // C.SWSP here, decoded as SW
   {0xC006, SW{Rs{2}, Rs{1}, 0}},
   // C.SDSP here, decoded as SD
@@ -350,6 +356,11 @@
   {0xE006, FSW{Rs{2}, Rs{1}, 0}},
   {0x6000, FLW{Rd{8}, Rs{8}, 0}},
   {0xE000, FSW{Rs{8}, Rs{8}, 0}},
+
+  {0x2084, FLD{Rd{9}, Rs{9}, 0}},
+  {0xA084, FSD{Rs{9}, Rs{9}, 0}},
+  {0x2082, FLD{Rd{1}, Rs{2}, 0}},
+  {0xA006, FSD{Rs{2}, Rs{1}, 0}},
   };
 
   for (auto i : tests) {
Index: lldb/source/Plugins/Instruction/RISCV/RISCVCInstructions.h
===
--- lldb/source/Plugins/Instruction/RISCV/RISCVCInstructions.h
+++ lldb/source/Plugins/Instruction/RISCV/RISCVCInstructions.h
@@ -327,5 +327,31 @@
   return FSW{Rs{gpr_sp_riscv}, DecodeCSS_RS2(inst), uint32_t(offset)};
 }
 
+RISCVInst DecodeC_FLDSP(uint32_t inst) {
+  auto rd = DecodeCI_RD(inst);
+  uint16_t offset = ((inst << 4) & 0x1c0)   // offset[8:6]
+| ((inst >> 7) & 0x20)  // offset[5]
+| ((inst >> 2) & 0x18); // offset[4:3]
+  return FLD{rd, Rs{gpr_sp_riscv}, uint32_t(offset)};
+}
+
+RISCVInst DecodeC_FSDSP(uint32_t inst) {
+  uint16_t offset = ((inst >> 1) & 0x1c0)   // offset[8:6]
+| ((inst >> 7) & 0x38); // offset[5:3]
+  return FSD{Rs{gpr_sp_riscv}, DecodeCSS_RS2(inst), uint32_t(offset)};
+}
+
+RISCVInst DecodeC_FLD(uint32_t inst) {
+  uint16_t offset = ((inst << 1) & 0xc0)// imm[7:6]
+| ((inst >> 7) & 0x38); // imm[5:3]
+  return FLD{DecodeCL_RD(inst), DecodeCL_RS1(inst), uint32_t(offset)};
+}
+
+RISCVInst DecodeC_FSD(uint32_t inst) {
+  uint16_t offset = ((inst << 1) & 0xc0)// imm[7:6]
+| ((inst >> 7) & 0x38); // imm[5:3]
+  return FSD{DecodeCS_RS1(inst), DecodeCS_RS2(inst), uint32_t(offset)};
+}
+
 } // namespace lldb_private
 #endif // LLDB_SOURCE_PLUGINS_INSTRUCTION_RISCV_RISCVCINSTRUCTION_H
Index: lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
===
--- lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
+++ lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
@@ -541,6 +541,11 @@
 {"FSW", 0xE003, 0xE000, DecodeC_FSW, RV32},
 {"FLWSP", 0xE003, 0x6002, DecodeC_FLWSP, RV32},
 {"FSWSP", 0xE003, 0xE002, DecodeC_FSWSP, RV32},
+// RVDC //
+{"FLDSP", 0xE003, 0x2002, DecodeC_FLDSP, RV32 | RV64},
+{"FSDSP", 0xE003, 0xA002, DecodeC_FSDSP, RV32 | RV64},
+{"FLD", 0xE003, 0x2000, DecodeC_FLD, RV32 | RV64},
+{"FSD", 0xE003, 0xA000, DecodeC_FSD, RV32 | RV64},
 
 // RV32F (Extension for Single-Precision Floating-Point) //
 {"FLW", 0x707F, 0x2007, DecodeIType},


Index: lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp
===
--- lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp
+++ lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp
@@ -287,8 +287,10 @@
   {0x0010, RESERVED{0x0010}},
   // ADDI4SPN here, decode as ADDI
   {0x0024, ADDI{Rd{9}, Rs{2}, 8}},
+  {0x2084, FLD{Rd{9}, Rs{9}, 0}},
   {0x4488, LW{Rd{10}, Rs{9}, 8}},
  

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman added a comment.

In D131858#4050112 , @rsmith wrote:

> In D131858#3957630 , @arphaman 
> wrote:
>
>> This change has caused a failure in Clang's stage 2 CI on the green dragon 
>> Darwin CI: 
>> https://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/6390/console.
>>
>>   Assertion failed: (lvaluePath->getType() == elemTy && "Unexpected type 
>> reference!"), function readAPValue, file 
>> /Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/tools/clang/include/clang/AST/AbstractBasicReader.inc,
>>  line 736.
>
> This assert is simply wrong, and I've removed it in 
> rG2009f2450532450a99c1a03d5e2c30f478121839 
>  -- that 
> change should be safe to cherry-pick into the release branch. It's possible 
> for the recomputation of the type after deserialization to result in a 
> different type than what we saw when serializing, because redeclarations of 
> the same entity can use the same type with different sugar -- or even 
> slightly different types in some cases, such as when an array bound is added 
> in a redeclaration. The dumps of the types provided by @steven_wu confirms 
> that we were just seeing a difference in type sugar in this case.
>
>>   Assertion failed: (BlockScope.empty() && CurAbbrevs.empty() && "Block 
>> imbalance"), function ~BitstreamWriter, file 
>> /Users/buildslave/jenkins/workspace/clang-stage1-RA/llvm-project/llvm/include/llvm/Bitstream/BitstreamWriter.h,
>>  line 119.
>
> Is this still happening? If so, this looks more serious, and will need 
> further investigation.
>
> Can we undo the workaround in https://reviews.llvm.org/D139956 and see if the 
> bot is now happy? Or can someone who was seeing problems before (@steven_wu?) 
> run a test?

Thank you for poking at this Richard! However, I think we still need to revert 
the functionality in this area unless we're able to make headway on 
https://github.com/llvm/llvm-project/issues/59271 and quickly. FWIW, I ran into 
this exact problem yesterday on my dev machine, so the overhead is still a 
present concern. If that's something you plan to work on, then I think it'd 
make sense for Erich to hold off on starting the revert work to give you a 
chance to improve this. But if nobody is actively working on it, we need to 
start pulling this back because the branch date is a bit over a week away (Jan 
24).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131858/new/

https://reviews.llvm.org/D131858

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

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

[Erich Keane via Phabricator via lldb-commits]

erichkeane added a comment.

In D131858#4051336 , @aaron.ballman 
wrote:

> In D131858#4050112 , @rsmith wrote:
>
>> In D131858#3957630 , @arphaman 
>> wrote:
>>
>>> This change has caused a failure in Clang's stage 2 CI on the green dragon 
>>> Darwin CI: 
>>> https://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/6390/console.
>>>
>>>   Assertion failed: (lvaluePath->getType() == elemTy && "Unexpected type 
>>> reference!"), function readAPValue, file 
>>> /Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/tools/clang/include/clang/AST/AbstractBasicReader.inc,
>>>  line 736.
>>
>> This assert is simply wrong, and I've removed it in 
>> rG2009f2450532450a99c1a03d5e2c30f478121839 
>>  -- 
>> that change should be safe to cherry-pick into the release branch. It's 
>> possible for the recomputation of the type after deserialization to result 
>> in a different type than what we saw when serializing, because 
>> redeclarations of the same entity can use the same type with different sugar 
>> -- or even slightly different types in some cases, such as when an array 
>> bound is added in a redeclaration. The dumps of the types provided by 
>> @steven_wu confirms that we were just seeing a difference in type sugar in 
>> this case.
>>
>>>   Assertion failed: (BlockScope.empty() && CurAbbrevs.empty() && "Block 
>>> imbalance"), function ~BitstreamWriter, file 
>>> /Users/buildslave/jenkins/workspace/clang-stage1-RA/llvm-project/llvm/include/llvm/Bitstream/BitstreamWriter.h,
>>>  line 119.
>>
>> Is this still happening? If so, this looks more serious, and will need 
>> further investigation.
>>
>> Can we undo the workaround in https://reviews.llvm.org/D139956 and see if 
>> the bot is now happy? Or can someone who was seeing problems before 
>> (@steven_wu?) run a test?
>
> Thank you for poking at this Richard! However, I think we still need to 
> revert the functionality in this area unless we're able to make headway on 
> https://github.com/llvm/llvm-project/issues/59271 and quickly. FWIW, I ran 
> into this exact problem yesterday on my dev machine, so the overhead is still 
> a present concern. If that's something you plan to work on, then I think it'd 
> make sense for Erich to hold off on starting the revert work to give you a 
> chance to improve this. But if nobody is actively working on it, we need to 
> start pulling this back because the branch date is a bit over a week away 
> (Jan 24).

My understanding is that the submitter of that bug did sufficient analysis to 
determine that https://reviews.llvm.org/D136566 is the cause of his perf 
regression, having done an analysis the patch before and after.  The only 
reason to revert THIS patch (and the follow-ups, since this is a 'base patch' 
to the rest) is the report by @steven_wu .

SO, @steven_wu: Can ypu please, ASAP, try to reproduce your issue as Richard 
asked above? IF so, we only have to revert D136566 
, which should fix our performance issue. 
(that is, revert the workaround you submitted in 
https://reviews.llvm.org/D139956, then see if it works?).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131858/new/

https://reviews.llvm.org/D131858

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

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

[Erich Keane via Phabricator via lldb-commits]

erichkeane added a comment.

@arphaman I see you're part of the original report too, if you could try to 
revert the workaround and test it, it would be nice too!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131858/new/

https://reviews.llvm.org/D131858

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

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman added a comment.

In D131858#4051352 , @erichkeane 
wrote:

> In D131858#4051336 , @aaron.ballman 
> wrote:
>
>> In D131858#4050112 , @rsmith wrote:
>>
>>> In D131858#3957630 , @arphaman 
>>> wrote:
>>>
 This change has caused a failure in Clang's stage 2 CI on the green dragon 
 Darwin CI: 
 https://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/6390/console.

   Assertion failed: (lvaluePath->getType() == elemTy && "Unexpected type 
 reference!"), function readAPValue, file 
 /Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/tools/clang/include/clang/AST/AbstractBasicReader.inc,
  line 736.
>>>
>>> This assert is simply wrong, and I've removed it in 
>>> rG2009f2450532450a99c1a03d5e2c30f478121839 
>>>  -- 
>>> that change should be safe to cherry-pick into the release branch. It's 
>>> possible for the recomputation of the type after deserialization to result 
>>> in a different type than what we saw when serializing, because 
>>> redeclarations of the same entity can use the same type with different 
>>> sugar -- or even slightly different types in some cases, such as when an 
>>> array bound is added in a redeclaration. The dumps of the types provided by 
>>> @steven_wu confirms that we were just seeing a difference in type sugar in 
>>> this case.
>>>
   Assertion failed: (BlockScope.empty() && CurAbbrevs.empty() && "Block 
 imbalance"), function ~BitstreamWriter, file 
 /Users/buildslave/jenkins/workspace/clang-stage1-RA/llvm-project/llvm/include/llvm/Bitstream/BitstreamWriter.h,
  line 119.
>>>
>>> Is this still happening? If so, this looks more serious, and will need 
>>> further investigation.
>>>
>>> Can we undo the workaround in https://reviews.llvm.org/D139956 and see if 
>>> the bot is now happy? Or can someone who was seeing problems before 
>>> (@steven_wu?) run a test?
>>
>> Thank you for poking at this Richard! However, I think we still need to 
>> revert the functionality in this area unless we're able to make headway on 
>> https://github.com/llvm/llvm-project/issues/59271 and quickly. FWIW, I ran 
>> into this exact problem yesterday on my dev machine, so the overhead is 
>> still a present concern. If that's something you plan to work on, then I 
>> think it'd make sense for Erich to hold off on starting the revert work to 
>> give you a chance to improve this. But if nobody is actively working on it, 
>> we need to start pulling this back because the branch date is a bit over a 
>> week away (Jan 24).
>
> My understanding is that the submitter of that bug did sufficient analysis to 
> determine that https://reviews.llvm.org/D136566 is the cause of his perf 
> regression, having done an analysis the patch before and after.  The only 
> reason to revert THIS patch (and the follow-ups, since this is a 'base patch' 
> to the rest) is the report by @steven_wu .

A thank you for the extra information. I had missed that this was one 
before the perf issue.

> SO, @steven_wu: Can ypu please, ASAP, try to reproduce your issue as Richard 
> asked above? IF so, we only have to revert D136566 
> , which should fix our performance issue. 
> (that is, revert the workaround you submitted in 
> https://reviews.llvm.org/D139956, then see if it works?).

+1, but based on the link to the workaround and what Richard fixed, I'm 
optimistic we can keep this patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131858/new/

https://reviews.llvm.org/D131858

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

[Lldb-commits] [PATCH] D141687: [LLDB] Remove return value from DumpRegisterValue

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

No one ever checks it. Also convert to early return.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141687

Files:
  lldb/include/lldb/Core/DumpRegisterValue.h
  lldb/source/Core/DumpRegisterValue.cpp

Index: lldb/source/Core/DumpRegisterValue.cpp
===
--- lldb/source/Core/DumpRegisterValue.cpp
+++ lldb/source/Core/DumpRegisterValue.cpp
@@ -15,66 +15,65 @@
 
 using namespace lldb;
 
-bool lldb_private::DumpRegisterValue(const RegisterValue ®_val, Stream *s,
+void lldb_private::DumpRegisterValue(const RegisterValue ®_val, Stream *s,
  const RegisterInfo *reg_info,
  bool prefix_with_name,
  bool prefix_with_alt_name, Format format,
  uint32_t reg_name_right_align_at,
  ExecutionContextScope *exe_scope) {
   DataExtractor data;
-  if (reg_val.GetData(data)) {
-bool name_printed = false;
-// For simplicity, alignment of the register name printing applies only in
-// the most common case where:
-//
-// prefix_with_name^prefix_with_alt_name is true
-//
-StreamString format_string;
-if (reg_name_right_align_at && (prefix_with_name ^ prefix_with_alt_name))
-  format_string.Printf("%%%us", reg_name_right_align_at);
-else
-  format_string.Printf("%%s");
-std::string fmt = std::string(format_string.GetString());
-if (prefix_with_name) {
-  if (reg_info->name) {
-s->Printf(fmt.c_str(), reg_info->name);
-name_printed = true;
-  } else if (reg_info->alt_name) {
-s->Printf(fmt.c_str(), reg_info->alt_name);
-prefix_with_alt_name = false;
-name_printed = true;
-  }
-}
-if (prefix_with_alt_name) {
-  if (name_printed)
-s->PutChar('/');
-  if (reg_info->alt_name) {
-s->Printf(fmt.c_str(), reg_info->alt_name);
-name_printed = true;
-  } else if (!name_printed) {
-// No alternate name but we were asked to display a name, so show the
-// main name
-s->Printf(fmt.c_str(), reg_info->name);
-name_printed = true;
-  }
+  if (!reg_val.GetData(data))
+return;
+
+  bool name_printed = false;
+  // For simplicity, alignment of the register name printing applies only in
+  // the most common case where:
+  //
+  // prefix_with_name^prefix_with_alt_name is true
+  //
+  StreamString format_string;
+  if (reg_name_right_align_at && (prefix_with_name ^ prefix_with_alt_name))
+format_string.Printf("%%%us", reg_name_right_align_at);
+  else
+format_string.Printf("%%s");
+  std::string fmt = std::string(format_string.GetString());
+  if (prefix_with_name) {
+if (reg_info->name) {
+  s->Printf(fmt.c_str(), reg_info->name);
+  name_printed = true;
+} else if (reg_info->alt_name) {
+  s->Printf(fmt.c_str(), reg_info->alt_name);
+  prefix_with_alt_name = false;
+  name_printed = true;
 }
+  }
+  if (prefix_with_alt_name) {
 if (name_printed)
-  s->PutCString(" = ");
+  s->PutChar('/');
+if (reg_info->alt_name) {
+  s->Printf(fmt.c_str(), reg_info->alt_name);
+  name_printed = true;
+} else if (!name_printed) {
+  // No alternate name but we were asked to display a name, so show the
+  // main name
+  s->Printf(fmt.c_str(), reg_info->name);
+  name_printed = true;
+}
+  }
+  if (name_printed)
+s->PutCString(" = ");
 
-if (format == eFormatDefault)
-  format = reg_info->format;
+  if (format == eFormatDefault)
+format = reg_info->format;
 
-DumpDataExtractor(data, s,
-  0,// Offset in "data"
-  format,   // Format to use when dumping
-  reg_info->byte_size,  // item_byte_size
-  1,// item_count
-  UINT32_MAX,   // num_per_line
-  LLDB_INVALID_ADDRESS, // base_addr
-  0,// item_bit_size
-  0,// item_bit_offset
-  exe_scope);
-return true;
-  }
-  return false;
+  DumpDataExtractor(data, s,
+0,// Offset in "data"
+format,   // Format to use when dumping
+reg_info->byte_size,  // item_byte_size
+1,// item_count
+UINT32_MAX,   // num_per_line
+LLDB_INVALID_ADDRESS, // base_addr
+0,// item_bit_size
+   

[Lldb-commits] [PATCH] D138792: [AArch64] Improve TargetParser API

[Tomas Matheson via Phabricator via lldb-commits]

tmatheson added a comment.

The most recent versions of this patch contains squashed changes from these 
reviews:

- D139278  "[AArch64] Use string comparison 
for ArchInfo equality." This fixes the test failures with shared libraries, 
which were caused by each shared library ending up with it's own copy of the 
ArchInfo instances and hence breaking the equality-by-address.
- D139102  "[AArch64] Inline 
AArch64TargetParser.def"

The `ArchInfo` class is no longer non-copyable to satisfy C++20. The 
alternative of adding a constructor would have resulted in global constructor 
calls, which is not allowed in Support (the same presumably applies to the new 
TargetParser library).

I've tested locally with `LLVM_BUILD_LLVM_DYLIB=ON` + `LLVM_LINK_LLVM_DYLIB=ON` 
+ `CMAKE_CXX_STANDARD=20`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138792/new/

https://reviews.llvm.org/D138792

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

[Lldb-commits] [PATCH] D138792: [AArch64] Improve TargetParser API

[Tomas Matheson via Phabricator via lldb-commits]

tmatheson added a comment.

Worth noting that this had to be reworked because both D127812 
 and D137838 
 went in since this was reverted.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138792/new/

https://reviews.llvm.org/D138792

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

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

[Erich Keane via Phabricator via lldb-commits]

erichkeane added a comment.

Note, I just did the revert of the workaround myself here: 
498bf3424d011235d420e02e80e135fae646d537 


I won't see the failure on 
https://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/ until Tuesday, but 
if it DOES fail like reported above, I'll continue on the effort to revert this 
patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131858/new/

https://reviews.llvm.org/D131858

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

[Lldb-commits] [PATCH] D138792: [AArch64] Improve TargetParser API

[Lucas Prates via Phabricator via lldb-commits]

pratlucas accepted this revision.
pratlucas added a comment.

LGTM with a tiny nit. Feel free to fix it when landing the changes.




Comment at: llvm/include/llvm/TargetParser/AArch64TargetParser.h:164-166
+  CPUFeatures CPUFeature;  // ?
+  StringRef DependentFeatures; // ?
+  unsigned FmvPriority;// ?

Minor nit: can you replace the `// ?` comments with a single TODO to 
document those better? It'd look a bit less confusing for others reading this 
in the future.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138792/new/

https://reviews.llvm.org/D138792

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

[Lldb-commits] [lldb] bafa145 - [lldb][NFC] Remove dependency on DataLayout::getPrefTypeAlignment

[Guillaume Chatelet via lldb-commits]

Author: Guillaume Chatelet
Date: 2023-01-13T15:04:06Z
New Revision: bafa145c0eb63ac0c7b662bd646aa2c73cbc07b7

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

LOG: [lldb][NFC] Remove dependency on DataLayout::getPrefTypeAlignment

Added: 


Modified: 
lldb/source/Expression/IRInterpreter.cpp

Removed: 




diff  --git a/lldb/source/Expression/IRInterpreter.cpp 
b/lldb/source/Expression/IRInterpreter.cpp
index 8e09e9f022442..36a8a74ed9efb 100644
--- a/lldb/source/Expression/IRInterpreter.cpp
+++ b/lldb/source/Expression/IRInterpreter.cpp
@@ -408,7 +408,7 @@ class InterpreterStackFrame {
 lldb_private::Status alloc_error;
 
 return Malloc(m_target_data.getTypeAllocSize(type),
-  m_target_data.getPrefTypeAlignment(type));
+  m_target_data.getPrefTypeAlign(type).value());
   }
 
   std::string PrintData(lldb::addr_t addr, llvm::Type *type) {



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

[Lldb-commits] [PATCH] D139249: [lldb] Add Debugger & ScriptedMetadata reference to Platform::CreateInstance

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In addition to that, it breaks tests with ubsan. :)

I raised a similar concern in one of the other patches. I'm of the opinion that 
it's not worth pretending like the scripted processes (platforms, etc.) are 
plugins. I don't think that it can be achieved without some sort of a leakage, 
and I don't think it makes sense given that the part that's really interesting 
to have pluggable is being able to plug a different scripting language into the 
scripted process. I don't exactly have an opinion on this metadata argument, 
but it definitely looks sub-optimal.

Given this uncertainty, and the ubsan problem, maybe we can revert this for the 
time being?




Comment at: lldb/unittests/Platform/PlatformTest.cpp:90
+   []() { Debugger::Initialize(nullptr); });
+list = new PlatformList(*m_debugger_sp.get());
+  }

m_debugger_sp is never initialized. I guess this kinda works because the test 
does not actually dereference it, but UBSAN lights it up straight away.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139249/new/

https://reviews.llvm.org/D139249

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

[Lldb-commits] [PATCH] D138792: [AArch64] Improve TargetParser API

[Daniel Kiss via Phabricator via lldb-commits]

danielkiss added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.cpp:532
 getTargetDefinesARMV81A(Opts, Builder);
-break;
-  case llvm::AArch64::ArchKind::ARMV8_2A:
+  if (*ArchInfo == llvm::AArch64::ARMV8_2A)
 getTargetDefinesARMV82A(Opts, Builder);





Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:172
+   *ArchInfo == llvm::AArch64::ARMV9_1A ||
+   *ArchInfo == llvm::AArch64::ARMV9_2A)) {
 Features.push_back("+sve");

Would be nice to add a custom operator to `ArchInfo` to say `*ArchInfo >= 
llvm::AArch64::ARMV9A`
because it looks to me here the `llvm::AArch64::ARMV9_3A` and 
`llvm::AArch64::ARMV9_4A` are missing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138792/new/

https://reviews.llvm.org/D138792

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

[Lldb-commits] [PATCH] D138792: [AArch64] Improve TargetParser API

[Tomas Matheson via Phabricator via lldb-commits]

tmatheson added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.cpp:532
 getTargetDefinesARMV81A(Opts, Builder);
-break;
-  case llvm::AArch64::ArchKind::ARMV8_2A:
+  if (*ArchInfo == llvm::AArch64::ARMV8_2A)
 getTargetDefinesARMV82A(Opts, Builder);

danielkiss wrote:
> 
I'll fix this when I push



Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:172
+   *ArchInfo == llvm::AArch64::ARMV9_1A ||
+   *ArchInfo == llvm::AArch64::ARMV9_2A)) {
 Features.push_back("+sve");

danielkiss wrote:
> Would be nice to add a custom operator to `ArchInfo` to say `*ArchInfo >= 
> llvm::AArch64::ARMV9A`
> because it looks to me here the `llvm::AArch64::ARMV9_3A` and 
> `llvm::AArch64::ARMV9_4A` are missing.
Good catch. This could be written as `if(ArchInfo.implies(ARMV9A))`, but I'll 
leave that for a follow up patch. I opted against a custom operator because 
they generally make things less understandable, except in cases where the 
ordering is very obvious, e.g. numeric types. For example 9.2 does not imply 
8.8. If you want to do an actual numerical comparison of version numbers you 
can compare ArchInfo.Versions directly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138792/new/

https://reviews.llvm.org/D138792

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

[Lldb-commits] [PATCH] D141658: [lldb/crashlog] Default interactive mode & report progress on image loading

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Switching the default over seems fine. The symbol lookup stuff seems orthogonal 
to that and maybe should be its own patch?




Comment at: lldb/examples/python/crashlog.py:452
 
-with open(path, 'r', encoding='utf-8') as f:
+with open(os.path.normpath(os.path.expanduser(path)), 'r', 
encoding='utf-8') as f:
 buffer = f.read()

It's not obvious why this is necessary (and we can't rely on open/the OS to do 
this for us) so let's add a comment. 



Comment at: lldb/examples/python/crashlog.py:1106-1108
+ci.HandleCommand('settings set symbols.enable-background-lookup true', 
result)
+if not result.Succeeded():
+raise InteractiveCrashLogException("couldn't enable background symbol 
lookup")

Background lookup is lazy: it's only going to start looking for symbols after 
we've determined they might be of interest. Right now that means after seeing 
an imagine in the backtrace. Without anything else, the first backtrace is 
still going be unsymbolicated (unless we had the symbols already available on 
the host).

It seems like you're doing the symbol loading explicitly in the C++ 
implementation of the scripted process, so what's depending on this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141658/new/

https://reviews.llvm.org/D141658

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

[Lldb-commits] [lldb] d667840 - Revert "[lldb] Add Debugger & ScriptedMetadata reference to Platform::CreateInstance"

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2023-01-13T09:13:03-08:00
New Revision: d667840465b0ea0dbd39dcbd56788e73698dc853

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

LOG: Revert "[lldb] Add Debugger & ScriptedMetadata reference to 
Platform::CreateInstance"

This reverts commit 2d53527e9c64c70c24e1abba74fa0a8c8b3392b1.

Added: 


Modified: 
lldb/bindings/interface/SBPlatform.i
lldb/include/lldb/API/SBDebugger.h
lldb/include/lldb/API/SBPlatform.h
lldb/include/lldb/API/SBStructuredData.h
lldb/include/lldb/Interpreter/OptionGroupPlatform.h
lldb/include/lldb/Target/Platform.h
lldb/include/lldb/lldb-private-interfaces.h
lldb/source/API/SBDebugger.cpp
lldb/source/API/SBPlatform.cpp
lldb/source/Commands/CommandObjectPlatform.cpp
lldb/source/Commands/CommandObjectPlatform.h
lldb/source/Core/Debugger.cpp
lldb/source/Interpreter/OptionGroupPlatform.cpp

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
lldb/source/Plugins/Platform/Android/PlatformAndroid.h
lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
lldb/source/Plugins/Platform/Linux/PlatformLinux.h
lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.h
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.h
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleWatch.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleWatch.h
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteiOS.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteiOS.h
lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
lldb/source/Plugins/Platform/OpenBSD/PlatformOpenBSD.cpp
lldb/source/Plugins/Platform/OpenBSD/PlatformOpenBSD.h
lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.h
lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
lldb/source/Plugins/Platform/Windows/PlatformWindows.h
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
lldb/source/Target/Platform.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/TargetList.cpp
lldb/unittests/Core/DiagnosticEventTest.cpp
lldb/unittests/Expression/DWARFExpressionTest.cpp
lldb/unittests/Interpreter/TestCommandPaths.cpp
lldb/unittests/Platform/PlatformAppleSimulatorTest.cpp
lldb/unittests/Platform/PlatformSiginfoTest.cpp
lldb/unittests/Platform/PlatformTest.cpp
lldb/unittests/Process/ProcessEventDataTest.cpp
lldb/unittests/Target/ExecutionContextTest.cpp
lldb/unittests/Target/StackFrameRecognizerTest.cpp
lldb/unittests/Thread/ThreadTest.cpp

Removed: 




diff  --git a/lldb/bindings/interface/SBPlatform.i 
b/lldb/bindings/interface/SBPlatform.i
index a57deca00e1d8..6413784e69e7c 100644
--- a/lldb/bindings/interface/SBPlatform.i
+++ b/lldb/bindings/interface/SBPlatform.i
@@ -126,18 +126,6 @@ public:
 
 SBPlatform (const char *);
 
-%feature("docstring", "
-Create a platform instance using a specific platform plugin name, debugger,
-script name and user-provided dictionary.
-
-:param platform_name: name of the platform plugin to use to create the 
platform
-:param debugger: debugger instance owning the platform
-:param script_name: name of the script class and module to use to create 
the platform
-:param dict: user-provided dictionary that can be used when instanciating 
a platform
-")
-SBPlatform (const char *, const SBDebugger&,
-const char *, const SBStructuredData&);
-
 ~SBPlatform();
 
 static SBPlatform GetHostPlatform();

diff  --git a/lldb/include/lldb/API/SBDebugger.h 

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: JDevlieghere, bulbazord.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch changes the way modules get loaded in lldb when using
interactive crashlog. It takes advantage of a Scripted Process
affordance to let lldb fetch and load modules itself.

This also allows us to report progress on module loading to the user.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141702

Files:
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -11,7 +11,7 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
-
+#include "lldb/Core/Progress.h"
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
@@ -19,6 +19,7 @@
 #include "lldb/Interpreter/OptionGroupBoolean.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Interpreter/ScriptedMetadata.h"
+#include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/Queue.h"
 #include "lldb/Target/RegisterContext.h"
@@ -410,15 +411,18 @@
 
   StructuredData::ArraySP loaded_images_sp = GetInterface().GetLoadedImages();
 
-  if (!loaded_images_sp || !loaded_images_sp->GetSize())
+  size_t num_image_to_load = loaded_images_sp->GetSize();
+  if (!loaded_images_sp || !num_image_to_load)
 return ScriptedInterface::ErrorWithMessage(
 LLVM_PRETTY_FUNCTION, "No loaded images.", error);
 
   ModuleList module_list;
   Target &target = GetTarget();
 
-  auto reload_image = [&target, &module_list, &error_with_message](
-  StructuredData::Object *obj) -> bool {
+  Progress progress("Fetching external dependencies", num_image_to_load);
+  auto reload_image = [&target, &module_list, &error_with_message,
+   &progress](StructuredData::Object *obj) -> bool {
+progress.Increment();
 StructuredData::Dictionary *dict = obj->GetAsDictionary();
 
 if (!dict)
@@ -445,6 +449,13 @@
 }
 module_spec.GetArchitecture() = target.GetArchitecture();
 
+Status error;
+Symbols::DownloadObjectAndSymbolFile(module_spec, error, true);
+if (error.Fail() ||
+!FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+  return error_with_message(error.AsCString());
+}
+
 ModuleSP module_sp =
 target.GetOrCreateModule(module_spec, true /* notify */);
 
@@ -469,9 +480,11 @@
 if (!changed && !module_sp->GetObjectFile())
   return error_with_message("Couldn't set the load address for module.");
 
-dict->GetValueForKeyAsString("path", value);
-FileSpec objfile(value);
-module_sp->SetFileSpecAndObjectName(objfile, objfile.GetFilename());
+if (has_path) {
+  dict->GetValueForKeyAsString("path", value);
+  FileSpec objfile(value);
+  module_sp->SetFileSpecAndObjectName(objfile, objfile.GetFilename());
+}
 
 return module_list.AppendIfNeeded(module_sp);
   };
Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -31,12 +31,10 @@
 if image not in self.loaded_images:
 if image.uuid == uuid.UUID(int=0):
 continue
-err = image.add_module(self.target)
-if err:
-# Append to SBCommandReturnObject
-print(err)
-else:
-self.loaded_images.append(image)
+for section in image.section_infos:
+if section.start_addr and section.name == "__TEXT":
+self.loaded_images.append({"uuid": 
str(image.uuid),
+   "load_addr": 
section.start_addr})
 
 for thread in crash_log.threads:
 if self.load_all_images:


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -11,7 +11,7 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
-
+#inc

[Lldb-commits] [PATCH] D141658: [lldb/crashlog] Make interactive mode the new default

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 489042.
mib marked 2 inline comments as done.
mib retitled this revision from "[lldb/crashlog] Default interactive mode & 
report progress on image loading" to "[lldb/crashlog] Make interactive mode the 
new default".
mib edited the summary of this revision.
mib added a comment.

Address @JDevlieghere comments:

- Move module loading logic to separate patch D141702 

- Remove use of background symbol lookup


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141658/new/

https://reviews.llvm.org/D141658

Files:
  lldb/examples/python/crashlog.py
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_invalid_target.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test

Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
@@ -1,7 +1,7 @@
 # RUN: %clang_host -g %S/Inputs/test.c -o %t.out
 # RUN: cp %S/Inputs/a.out.crash %t.crash
 # RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":20, "bar":9, "foo":16}'
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog -b %t.crash' 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
 
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
@@ -3,7 +3,7 @@
 # RUN: mkdir -p %t.dir
 # RUN: yaml2obj %S/Inputs/interactive_crashlog/multithread-test.yaml > %t.dir/multithread-test
 # RUN: %lldb -b -o 'command script import lldb.macosx.crashlog' \
-# RUN: -o 'crashlog -a -i -s -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
+# RUN: -o 'crashlog -a -s -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
 # RUN: -o 'command source -s 0 %s' 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
@@ -3,7 +3,7 @@
 # RUN: mkdir -p %t.dir
 # RUN: yaml2obj %S/Inputs/interactive_crashlog/multithread-test.yaml > %t.dir/multithread-test
 # RUN: %lldb -o 'command script import lldb.macosx.crashlog' \
-# RUN: -o 'crashlog -a -i -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
+# RUN: -o 'crashlog -a -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
 # RUN: -o "thread list" -o "bt all" 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
@@ -2,7 +2,7 @@
 
 # RUN: cp %S/Inputs/no_threadState.ips %t.crash
 # RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":20, "bar":9, "foo":16}' --json
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog -b %t.crash' 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
 
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
@@ -2,11 +2,11 @@
 
 # RUN: cp %S/Inputs/a.out.ips %t.crash
 # RUN: %python %S/patch-crashlo

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

[Vassil Vassilev via Phabricator via lldb-commits]

v.g.vassilev added a comment.

Thanks a lot @rsmith for providing a fix and thanks a lot @aaron.ballman and 
@erichkeane for the efforts saving @mizvekov work over the summer. I believe he 
has sporadic access to internet and soon he will be back to normal. Great 
example of team work here!!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131858/new/

https://reviews.llvm.org/D131858

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

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

[Erich Keane via Phabricator via lldb-commits]

erichkeane added a comment.

In D131858#4052026 , @v.g.vassilev 
wrote:

> Thanks a lot @rsmith for providing a fix and thanks a lot @aaron.ballman and 
> @erichkeane for the efforts saving @mizvekov work over the summer. I believe 
> he has sporadic access to internet and soon he will be back to normal. Great 
> example of team work here!!

Note we don't yet have the evidence that this is savable yet, we should know in 
the next day or so.  At minimum, at least 1 of his patches needs to be reverted 
due to the memory regression.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131858/new/

https://reviews.llvm.org/D131858

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

[Lldb-commits] [PATCH] D141658: [lldb/crashlog] Make interactive mode the new default

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Another nice addition here would be checking if stdout is a tty 
(`sys.stdout.isatty()`) and continuing to use the textual format if it isn't. A 
quick experiment seems to show this does the right thing from inside LLDB:

  $ echo -e "script\nimport sys\nsys.stdout.isatty()" | lldb -b
  (lldb) script
  Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
  >>> >>> True
  now exiting InteractiveConsole...
  $ echo -e "script\nimport sys\nsys.stdout.isatty()" | lldb -b | tail
  Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
  
  now exiting InteractiveConsole...
  (lldb) script
  >>> >>> False




Comment at: lldb/examples/python/crashlog.py:1302
+print("Aborting symbolication.")
+print()
+option_parser.print_help()

Spurious print? It doesn't look like we're printing a double newline elsewhere.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141658/new/

https://reviews.llvm.org/D141658

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

[Lldb-commits] [PATCH] D141658: [lldb/crashlog] Make interactive mode the new default

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: lldb/examples/python/crashlog.py:1302
+print("Aborting symbolication.")
+print()
+option_parser.print_help()

JDevlieghere wrote:
> Spurious print? It doesn't look like we're printing a double newline 
> elsewhere.
I wanted to leave space between the actual error message and the help



Comment at: lldb/examples/python/crashlog.py:452
 
-with open(path, 'r', encoding='utf-8') as f:
+with open(os.path.normpath(os.path.expanduser(path)), 'r', 
encoding='utf-8') as f:
 buffer = f.read()

JDevlieghere wrote:
> It's not obvious why this is necessary (and we can't rely on open/the OS to 
> do this for us) so let's add a comment. 
From my experience, `open`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141658/new/

https://reviews.llvm.org/D141658

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

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

I'm somewhat unfamiliar with this code so I'll ask some clarifying questions. I 
think I understand the idea in general though.




Comment at: 
lldb/examples/python/scripted_process/crashlog_scripted_process.py:34-37
+for section in image.section_infos:
+if section.start_addr and section.name == "__TEXT":
+self.loaded_images.append({"uuid": 
str(image.uuid),
+   "load_addr": 
section.start_addr})

I don't understand the intent of this part. It looks like you're changing the 
format of `self.loaded_images` here. It's still a List, but instead of 
containing images it contains specific information about specific sections of 
each image. If the format has changed, don't consumers of `get_loaded_images` 
need to be modified as well?



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:452-458
+Status error;
+Symbols::DownloadObjectAndSymbolFile(module_spec, error, true);
+if (error.Fail() ||
+!FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+  return error_with_message(error.AsCString());
+}
+

Shouldn't you be using the result of `Symbols::DownloadObjectAndSymbolFile` 
here? I've read through the implementation of `DownloadObjectAndSymbolFile` and 
here's what I've surmised:

- The non-Darwin implementation of `DownloadObjectAndSymbolFile` simply returns 
false, so `error` won't be populated.
- The Darwin implementation fills in `error` only as a result of invoking 
`dsymForUUID` and subsequent processing of the result. It's entirely possible 
for the function to return false before `dsymForUUID` is invoked or without any 
useful information in `error`.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141702/new/

https://reviews.llvm.org/D141702

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

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

Additionally, it looks like you're doing maybe 2-3 things in this patch 
(Progress reporting, adding a guard rail, changing the format of 
`loaded_images` in the scripted process example). You could probably break this 
up into multiple patches.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141702/new/

https://reviews.llvm.org/D141702

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

[Lldb-commits] [lldb] ed01de6 - [OpenMP][OMPIRBuilder] Move SIMD alignment calculation to LLVM Frontend

[Dominik Adamski via lldb-commits]

Author: Dominik Adamski
Date: 2023-01-13T14:07:29-06:00
New Revision: ed01de67433174d3157e9d239d59dd465d52c6a5

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

LOG: [OpenMP][OMPIRBuilder] Move SIMD alignment calculation to LLVM Frontend

Currently default simd alignment is specified by Clang specific TargetInfo
class. This class cannot be reused for LLVM Flang. If we move the default
alignment field into TargetMachine class then we can create TargetMachine
objects and query them to find SIMD alignment.

Scope of changes:
  1) Added information about maximal allowed SIMD alignment to TargetMachine
 classes.
  2) Removed getSimdDefaultAlign function from Clang TargetInfo class.
  3) Refactored createTargetMachine function.

Reviewed By: jsjodin

Differential Revision: https://reviews.llvm.org/D138496

Added: 


Modified: 
clang/include/clang/Basic/TargetInfo.h
clang/lib/AST/ASTContext.cpp
clang/lib/Basic/TargetInfo.cpp
clang/lib/Basic/Targets/PPC.h
clang/lib/Basic/Targets/WebAssembly.h
clang/lib/Basic/Targets/X86.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
llvm/include/llvm/Target/TargetMachine.h
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
llvm/lib/Target/X86/X86TargetMachine.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/TargetInfo.h 
b/clang/include/clang/Basic/TargetInfo.h
index a5aea33d84751..def708daac8d2 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -226,7 +226,6 @@ class TargetInfo : public virtual TransferrableTargetInfo,
   bool HasStrictFP;
 
   unsigned char MaxAtomicPromoteWidth, MaxAtomicInlineWidth;
-  unsigned short SimdDefaultAlign;
   std::string DataLayoutString;
   const char *UserLabelPrefix;
   const char *MCountName;
@@ -795,10 +794,6 @@ class TargetInfo : public virtual TransferrableTargetInfo,
 
   /// Return the maximum vector alignment supported for the given target.
   unsigned getMaxVectorAlign() const { return MaxVectorAlign; }
-  /// Return default simd alignment for the given target. Generally, this
-  /// value is type-specific, but this alignment can be used for most of the
-  /// types for the given target.
-  unsigned getSimdDefaultAlign() const { return SimdDefaultAlign; }
 
   unsigned getMaxOpenCLWorkGroupSize() const { return MaxOpenCLWorkGroupSize; }
 

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 15a43807c3603..6b97407236ca5 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -79,6 +79,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
 #include "llvm/Support/Capacity.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
@@ -93,6 +94,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -2463,7 +2465,16 @@ unsigned ASTContext::getTypeUnadjustedAlign(const Type 
*T) const {
 }
 
 unsigned ASTContext::getOpenMPDefaultSimdAlign(QualType T) const {
-  unsigned SimdAlign = getTargetInfo().getSimdDefaultAlign();
+  const std::vector &TargetFeatures =
+  Target->getTargetOpts().Features;
+  std::string TargetFeaturesString = std::accumulate(
+  TargetFeatures.cbegin(), TargetFeatures.cend(), std::string(),
+  [](const std::string &s1, const std::string &s2) {
+return s1.empty() ? s2 : s1 + "," + s2;
+  });
+  unsigned SimdAlign = llvm::OpenMPIRBuilder ::getSimdDefaultAlignment(
+  getTargetInfo().getTriple().str(), Target->getTargetOpts().CPU,
+  TargetFeaturesString);
   return SimdAlign;
 }
 

diff  --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index 8ee43261fc1d3..fa5e568d599d0 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -119,7 +119,6 @@ TargetInfo::TargetInfo(const llvm::Triple &T) : Triple(T) {
   MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 0;
   MaxVectorAlign = 0;
   MaxTLSAlign = 0;
-  SimdDefaultAlign = 0;
   SizeType = UnsignedLong;
   PtrDiffType = SignedLong;
   IntMaxType = SignedLongLong;

diff  --git a/clang/lib/Basic/Targets/PPC.h b/clang/lib/Basic/Targets/PPC.h
index cc185fdadfcbc..4c02183feb4c1 100644
--- a/clang/lib/Basic/Targets/PPC.h
+++ b/clang/lib/Basic/Targets/PPC.h
@@ -87,7 +87,6 @@ class LLVM_LIBRARY_VISIBILITY PPCTargetInfo : public 
TargetInfo {
   PPCTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
   : TargetInfo(Triple) {
 SuitableAlign = 128;
-SimdDefaultAlign = 128;
 LongDoubleWidth =

[Lldb-commits] [PATCH] D138496: [OpenMP][OMPContext] Move SIMD alignment calculation to LLVM Frontend

[Dominik Adamski via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGed01de674331: [OpenMP][OMPIRBuilder] Move SIMD alignment 
calculation to LLVM Frontend (authored by domada).
Herald added projects: clang, LLDB.
Herald added subscribers: lldb-commits, cfe-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138496/new/

https://reviews.llvm.org/D138496

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Target/TargetMachine.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp

Index: llvm/lib/Target/X86/X86TargetMachine.cpp
===
--- llvm/lib/Target/X86/X86TargetMachine.cpp
+++ llvm/lib/Target/X86/X86TargetMachine.cpp
@@ -245,6 +245,13 @@
   setSupportsDebugEntryValues(true);
 
   initAsmInfo();
+
+  if (FS.contains("+avx512f"))
+SimdDefaultAlignment = 512;
+  else if (FS.contains("+avx"))
+SimdDefaultAlignment = 256;
+  else
+SimdDefaultAlignment = 128;
 }
 
 X86TargetMachine::~X86TargetMachine() = default;
Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -139,6 +139,7 @@
   this->Options.FunctionSections = true;
   this->Options.DataSections = true;
   this->Options.UniqueSectionNames = true;
+  SimdDefaultAlignment = 128;
 
   initAsmInfo();
 
Index: llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
===
--- llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
+++ llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
@@ -333,6 +333,7 @@
   TLOF(createTLOF(getTargetTriple())),
   TargetABI(computeTargetABI(TT, Options)),
   Endianness(isLittleEndianTriple(TT) ? Endian::LITTLE : Endian::BIG) {
+  SimdDefaultAlignment = 128;
   initAsmInfo();
 }
 
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -255,6 +255,50 @@
   NewBr->setDebugLoc(DL);
 }
 
+/// Create the TargetMachine object to query the backend for optimization
+/// preferences.
+///
+/// Ideally, this would be passed from the front-end to the OpenMPBuilder, but
+/// e.g. Clang does not pass it to its CodeGen layer and creates it only when
+/// needed for the LLVM pass pipline. We use some default options to avoid
+/// having to pass too many settings from the frontend that probably do not
+/// matter.
+///
+/// Currently, TargetMachine is only used sometimes by the unrollLoopPartial
+/// and getVectorTypeAlignment methods. If we are going to use TargetMachine
+/// for more purposes, especially those that are sensitive to TargetOptions,
+/// RelocModel and CodeModel, it might become be worth requiring front-ends to
+/// pass on their TargetMachine, or at least cache it between methods.
+/// Note that while fontends such as Clang have just a single main
+/// TargetMachine per translation unit, "target-cpu" and "target-features"
+/// that determine the TargetMachine are per-function and can be overrided
+/// using __attribute__((target("OPTIONS"))).
+static std::unique_ptr
+createTargetMachine(const std::string &Triple, StringRef CPU,
+StringRef Features, CodeGenOpt::Level OptLevel) {
+  std::string Error;
+  const llvm::Target *TheTarget = TargetRegistry::lookupTarget(Triple, Error);
+  if (!TheTarget)
+return {};
+
+  llvm::TargetOptions Options;
+  return std::unique_ptr(TheTarget->createTargetMachine(
+  Triple, CPU, Features, Options, /*RelocModel=*/std::nullopt,
+  /*CodeModel=*/std::nullopt, OptLevel));
+}
+
+/// Create the TargetMachine object to query the backend for optimization
+/// preferences.
+static std::unique_ptr
+createTargetMachine(Function *F, CodeGenOpt::Level OptLevel) {
+  Module *M = F->getParent();
+
+  StringRef CPU = F->getFnAttribute("target-cpu").getValueAsString();
+  StringRef Features = F->getFnAttribute("target-features").getValueAsString();
+  const std::string &Triple = M->getTargetTriple();
+  return createTargetMachine(Triple, CPU, Features, OptLevel);
+}
+
 void llvm::spliceBB(IRBuilderBase::InsertPoint IP, BasicBlock *New,
 bool CreateBranch) {
   assert(New->getFirstInsertionPt() == New->begin() &&
@@

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

[Med Ismail Bennani via Phabricator via lldb-commits]

mib marked an inline comment as done.
mib added inline comments.



Comment at: 
lldb/examples/python/scripted_process/crashlog_scripted_process.py:34-37
+for section in image.section_infos:
+if section.start_addr and section.name == "__TEXT":
+self.loaded_images.append({"uuid": 
str(image.uuid),
+   "load_addr": 
section.start_addr})

bulbazord wrote:
> I don't understand the intent of this part. It looks like you're changing the 
> format of `self.loaded_images` here. It's still a List, but instead of 
> containing images it contains specific information about specific sections of 
> each image. If the format has changed, don't consumers of `get_loaded_images` 
> need to be modified as well?
The list for this specific scripted process class was wrong, it expects the 
current format. Previously we didn't make use of `loaded_images` for the 
crashlog_scripted_process class, we just relied on some ad hoc heuristic to 
load the modules. 
The other consumers of `get_loaded_images` are fine.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:452-458
+Status error;
+Symbols::DownloadObjectAndSymbolFile(module_spec, error, true);
+if (error.Fail() ||
+!FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+  return error_with_message(error.AsCString());
+}
+

bulbazord wrote:
> Shouldn't you be using the result of `Symbols::DownloadObjectAndSymbolFile` 
> here? I've read through the implementation of `DownloadObjectAndSymbolFile` 
> and here's what I've surmised:
> 
> - The non-Darwin implementation of `DownloadObjectAndSymbolFile` simply 
> returns false, so `error` won't be populated.
> - The Darwin implementation fills in `error` only as a result of invoking 
> `dsymForUUID` and subsequent processing of the result. It's entirely possible 
> for the function to return false before `dsymForUUID` is invoked or without 
> any useful information in `error`.
> 
> 
Good catch!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141702/new/

https://reviews.llvm.org/D141702

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

[Lldb-commits] [lldb] 6809af1 - Revert "[OpenMP][OMPIRBuilder] Move SIMD alignment calculation to LLVM Frontend"

[Dominik Adamski via lldb-commits]

Author: Dominik Adamski
Date: 2023-01-13T14:38:17-06:00
New Revision: 6809af1a232bc5ac71358e4b874759ddaae056a1

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

LOG: Revert "[OpenMP][OMPIRBuilder] Move SIMD alignment calculation to LLVM 
Frontend"

This reverts commit ed01de67433174d3157e9d239d59dd465d52c6a5.

Added: 


Modified: 
clang/include/clang/Basic/TargetInfo.h
clang/lib/AST/ASTContext.cpp
clang/lib/Basic/TargetInfo.cpp
clang/lib/Basic/Targets/PPC.h
clang/lib/Basic/Targets/WebAssembly.h
clang/lib/Basic/Targets/X86.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
llvm/include/llvm/Target/TargetMachine.h
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
llvm/lib/Target/X86/X86TargetMachine.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/TargetInfo.h 
b/clang/include/clang/Basic/TargetInfo.h
index def708daac8d2..a5aea33d84751 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -226,6 +226,7 @@ class TargetInfo : public virtual TransferrableTargetInfo,
   bool HasStrictFP;
 
   unsigned char MaxAtomicPromoteWidth, MaxAtomicInlineWidth;
+  unsigned short SimdDefaultAlign;
   std::string DataLayoutString;
   const char *UserLabelPrefix;
   const char *MCountName;
@@ -794,6 +795,10 @@ class TargetInfo : public virtual TransferrableTargetInfo,
 
   /// Return the maximum vector alignment supported for the given target.
   unsigned getMaxVectorAlign() const { return MaxVectorAlign; }
+  /// Return default simd alignment for the given target. Generally, this
+  /// value is type-specific, but this alignment can be used for most of the
+  /// types for the given target.
+  unsigned getSimdDefaultAlign() const { return SimdDefaultAlign; }
 
   unsigned getMaxOpenCLWorkGroupSize() const { return MaxOpenCLWorkGroupSize; }
 

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 6b97407236ca5..15a43807c3603 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -79,7 +79,6 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
-#include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
 #include "llvm/Support/Capacity.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
@@ -94,7 +93,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -2465,16 +2463,7 @@ unsigned ASTContext::getTypeUnadjustedAlign(const Type 
*T) const {
 }
 
 unsigned ASTContext::getOpenMPDefaultSimdAlign(QualType T) const {
-  const std::vector &TargetFeatures =
-  Target->getTargetOpts().Features;
-  std::string TargetFeaturesString = std::accumulate(
-  TargetFeatures.cbegin(), TargetFeatures.cend(), std::string(),
-  [](const std::string &s1, const std::string &s2) {
-return s1.empty() ? s2 : s1 + "," + s2;
-  });
-  unsigned SimdAlign = llvm::OpenMPIRBuilder ::getSimdDefaultAlignment(
-  getTargetInfo().getTriple().str(), Target->getTargetOpts().CPU,
-  TargetFeaturesString);
+  unsigned SimdAlign = getTargetInfo().getSimdDefaultAlign();
   return SimdAlign;
 }
 

diff  --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index fa5e568d599d0..8ee43261fc1d3 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -119,6 +119,7 @@ TargetInfo::TargetInfo(const llvm::Triple &T) : Triple(T) {
   MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 0;
   MaxVectorAlign = 0;
   MaxTLSAlign = 0;
+  SimdDefaultAlign = 0;
   SizeType = UnsignedLong;
   PtrDiffType = SignedLong;
   IntMaxType = SignedLongLong;

diff  --git a/clang/lib/Basic/Targets/PPC.h b/clang/lib/Basic/Targets/PPC.h
index 4c02183feb4c1..cc185fdadfcbc 100644
--- a/clang/lib/Basic/Targets/PPC.h
+++ b/clang/lib/Basic/Targets/PPC.h
@@ -87,6 +87,7 @@ class LLVM_LIBRARY_VISIBILITY PPCTargetInfo : public 
TargetInfo {
   PPCTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
   : TargetInfo(Triple) {
 SuitableAlign = 128;
+SimdDefaultAlign = 128;
 LongDoubleWidth = LongDoubleAlign = 128;
 LongDoubleFormat = &llvm::APFloat::PPCDoubleDouble();
 HasStrictFP = true;

diff  --git a/clang/lib/Basic/Targets/WebAssembly.h 
b/clang/lib/Basic/Targets/WebAssembly.h
index 1f0bb08665347..1e73450fdd0c3 100644
--- a/clang/lib/Basic/Targets/WebAssembly.h
+++ b/clang/lib/Basic/Targets/WebAssembly.h
@@ -49,6 +49,7 @@ class LLVM_LIBRARY_VISIBILITY WebAssemblyTargetInfo : public 
TargetInfo {
 SuitableAlign = 128;
 LargeArrayMinWidth = 128;
 LargeArrayAlign =

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 489102.
mib marked an inline comment as done.
mib added a reviewer: jasonmolenda.
mib added a comment.

Address @bulbazord feedback


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141702/new/

https://reviews.llvm.org/D141702

Files:
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -11,7 +11,7 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
-
+#include "lldb/Core/Progress.h"
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
@@ -19,6 +19,7 @@
 #include "lldb/Interpreter/OptionGroupBoolean.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Interpreter/ScriptedMetadata.h"
+#include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/Queue.h"
 #include "lldb/Target/RegisterContext.h"
@@ -410,15 +411,18 @@
 
   StructuredData::ArraySP loaded_images_sp = GetInterface().GetLoadedImages();
 
-  if (!loaded_images_sp || !loaded_images_sp->GetSize())
+  size_t num_image_to_load = loaded_images_sp->GetSize();
+  if (!loaded_images_sp || !num_image_to_load)
 return ScriptedInterface::ErrorWithMessage(
 LLVM_PRETTY_FUNCTION, "No loaded images.", error);
 
   ModuleList module_list;
   Target &target = GetTarget();
 
-  auto reload_image = [&target, &module_list, &error_with_message](
-  StructuredData::Object *obj) -> bool {
+  Progress progress("Fetching external dependencies", num_image_to_load);
+  auto reload_image = [&target, &module_list, &error_with_message,
+   &progress](StructuredData::Object *obj) -> bool {
+progress.Increment();
 StructuredData::Dictionary *dict = obj->GetAsDictionary();
 
 if (!dict)
@@ -445,6 +449,12 @@
 }
 module_spec.GetArchitecture() = target.GetArchitecture();
 
+Status error;
+if (!Symbols::DownloadObjectAndSymbolFile(module_spec, error, true) ||
+error.Fail() ||
+!FileSystem::Instance().Exists(module_spec.GetFileSpec()))
+  return error_with_message(error.AsCString());
+
 ModuleSP module_sp =
 target.GetOrCreateModule(module_spec, true /* notify */);
 
@@ -469,9 +479,11 @@
 if (!changed && !module_sp->GetObjectFile())
   return error_with_message("Couldn't set the load address for module.");
 
-dict->GetValueForKeyAsString("path", value);
-FileSpec objfile(value);
-module_sp->SetFileSpecAndObjectName(objfile, objfile.GetFilename());
+if (has_path) {
+  dict->GetValueForKeyAsString("path", value);
+  FileSpec objfile(value);
+  module_sp->SetFileSpecAndObjectName(objfile, objfile.GetFilename());
+}
 
 return module_list.AppendIfNeeded(module_sp);
   };
Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -31,12 +31,10 @@
 if image not in self.loaded_images:
 if image.uuid == uuid.UUID(int=0):
 continue
-err = image.add_module(self.target)
-if err:
-# Append to SBCommandReturnObject
-print(err)
-else:
-self.loaded_images.append(image)
+for section in image.section_infos:
+if section.start_addr and section.name == "__TEXT":
+self.loaded_images.append({"uuid": 
str(image.uuid),
+   "load_addr": 
section.start_addr})
 
 for thread in crash_log.threads:
 if self.load_all_images:


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -11,7 +11,7 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
-
+#include "lldb/Core/Progress.h"
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
@@ -19,6 +19,7 @@
 #include "lldb/Interpreter/OptionGroupBoolean.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Interpreter/Scripted

[Lldb-commits] [PATCH] D68655: Trust the arange accelerator tables in dSYMs

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D68655#4045873 , @jasonmolenda 
wrote:

> I know this is all moot because the dSYM-specific patch landed, but I am 
> curious about this part,
>
> In D68655#4045561 , @clayborg wrote:
>
>> 
>
>
>
>> Different things are included in DW_AT_ranges, like address ranges for 
>> global and static variables. .debug_aranges only has functions, no globals 
>> or statics, so if you are trying to find a global variable by address, you 
>> can't rely on .debug_aranges. Nothing in the DWARF spec states things 
>> clearly enough for different compilers to know what to include in 
>> .debug_aranges  and the compiler uint DW_AT_ranges.
>
> The standard says,
>
> "Each descriptor is a triple consisting of a segment selector, the beginning 
> address within that segment of a range of text or data covered by some entry 
> owned by the corresponding compilation unit, followed by the non-zero length 
> of that range"
>
> It is pretty clear on the point that any part of the address space that can 
> be attributed to a compile_unit can be included in the debug_aranges range 
> list - if only code is included, that's a choice of the aranges writer.  lldb 
> itself, if debug_aranges is missing or doesn't include a CU, steps through 
> the line table concatenating addresses for all the line entries in the CU (v. 
> DWARFCompileUnit::BuildAddressRangeTable ) - it doesn't include data.  (it 
> will also use DW_AT_ranges from the compile_unit but I worry more about this 
> being copied verbatim from the .o file into the linked DWARF than I worry 
> about debug_aranges, personally)

Sorry, looks like the spec is pretty clear. From what I remember David Blakie 
saying, clang won't put globals or static (no data, just code) into the 
.debug_aranges. GCC might do this differently. I thought that LLDB would try to 
use the following in order:

- .debug_aranges for a CU if available
- fall back to DW_AT_ranges
- fall back to line tables only if DW_AT_ranges is not there

> In a DW_TAG_compile_unit, the DW_AT_ranges that the compiler puts in the .o 
> file isn't relevant to the final linked debug information, unless it included 
> the discrete range of every item which might be linked into the final binary, 
> and the DWARF linker only copies the range entries that were in the final 
> binary in the linked dwarf DW_AT_ranges range list.   Or if the dwarf linker 
> knows how to scan the DWARF compile_unit like lldb does, concatenating line 
> table entries or DW_TAG_subprogram's.

The Darwin workflow has a smart DWARF linker, so yes, it will regenerate only 
what is needed for the final output in dsymutil. The compiler have been trained 
to not emit .debug_aranges for Darwin triples because we know we don't need 
them since dsymutil will regenerate from the final output only what is needed.

All other platforms use standard linker technology where they will concatenate 
all DWARF sections to create the final DWARF sections and then apply 
relocations. So if you have a DW_AT_ranges in a CU that has 100 addresses in a 
.o file, and the final output only uses 10 functions and dead strips 90 
functions, the final .debug_aranges for that CU will still contain ranges for 
all functions, 10 of which will have had relocations applied and will have 
valid address ranges, and 90 others will start at the sentinel addresss of zero 
or -1 to indicate they should have been dead stripped, but there can be tons of 
wasted data that isn't needed in each .debug_aranges CU data.

> If any dwarf linker is doing all of this work to construct an accurate & 
> comprehensive compile_unit DW_AT_ranges, why couldn't it be trusted to also 
> have an identical/accurate dwarf_aranges for this cu?  I don't see the 
> difference.

Only dsymutil does this as this is the only smart DWARF linker at the moment! 
See above comment as to why. We do have llvm-dwarfutil now for ELF files that 
can optimize DWARF after it has been linked to remove all of this extra junk 
and unique types. llvm-dwarfutil uses the same DWARF linking engine as 
dsymutil. But all other systems are concatenate all sections, then apply 
relocations, and anything that doesn't have a relocation gets a sentinel 
address applied as the relocation.

In D68655#4045895 , @jasonmolenda 
wrote:

> I guess to say it shorter.  If I have a dwarf_aranges, that means the dwarf 
> linker created it.  And if it created it, surely its at least based off of 
> the subprogram address ranges or the line table -- that is, the text address 
> ranges.

True for Darwin and dsymutil yes.

> If I have a DW_TAG_compile_unit DW_AT_ranges, either the compiler (to the .o 
> file) created it, in which case I really am suspicious of those ranges 
> because the compiler can't know which symbols will end up in the final 
> executable, and the addresses in the ranges were simply translate

[Lldb-commits] [PATCH] D68655: Trust the arange accelerator tables in dSYMs

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D68655#4048009 , @jasonmolenda 
wrote:

> In D68655#4047126 , @labath wrote:
>
>> In D68655#4045895 , @jasonmolenda 
>> wrote:
>>
>>> Both have to be written by the dwarf linker to be correct, but only the 
>>> former is written ONLY by the dwarf linker.
>>
>> I don't think that's right:
>>
>>   $ clang -c -x c - -o - -gdwarf-aranges -g <<<"void f(){}" | llvm-readelf - 
>> --sections | grep aranges
>> [ 6] .debug_arangesPROGBITS a0 30 00 
>>  0   0  1
>> [ 7] .rela.debug_aranges RELA   000380 30 18 
>>   I 20   6  8
>
> Ah thanks Pavel, clang on Darwin doesn't emit this in .o files.  So in this 
> case, the .o file has a DW_TAG_compile_unit with a DW_AT_ranges and a 
> .debug_aranges, both generated pre-link-time by the compiler.  And Greg is 
> saying that the DW_AT_ranges list in the final executable will be better than 
> the .debug_aranges?  I don't know how strongly he asserts this - today if a 
> .debug_aranges has an entry, we don't look at DW_TAG_compile_unit's 
> DW_AT_ranges, or create the ranges ourself via the line table; the 
> debug_aranges is trusted above the other methods if it includes CU.

It isn't better or worse and it varies per compiler. I believe, but I don't 
remember for sure, that DW_AT_ranges only currently has code address ranges (no 
data, so no globals or statics). So if a compiler in fact does include data in 
the .debug_aranges, it would be better to use .debug_aranges.

I tried to add a verifier into llvm-dwarfdump to catch when either 
.debug_aranges or DW_AT_ranges on the CU didn't contain some address ranges 
that were in the CU with:

https://reviews.llvm.org/D136395

The problem is the debugger has issues if either of these tables are incorrect, 
so you would think a verifier would be a good thing since if the address isn't 
in either .debug_aranges or DW_AT_ranges, we won't ever find a DIE for a 
function or global variable. I had seen bugs where we have .debug_aranges that 
didn't include all ranges for all DW_TAG_subprogram DIEs, and also cases where 
DW_AT_ranges didn't have some ranges. This usually happens when someone tries 
to LTO or optimize a binary and the DWARF didn't have a dedicate entry for a 
function, so it would drop address ranges from the .debug_aranges or 
DW_AT_ranges. Since everything non Darwin uses "concatenate + relocate" in the 
linker, image if you have a .debug_aranges entry that had [0x1000-0x2000), and 
you had DWARF with two functions that fall into those ranges: [0x1000-0x1500) 
for "func1" and [0x1500-0x2000) for "func2", and those functions get optimized 
into different places in the binary (not right next to each other), then there 
is no valid relocation to can apply to the range [0x1000-0x2000) that will make 
the table continue to work after linking. You will end up relocating the 0x1000 
address correctly for "func1", and if you apply the relocation for the end 
address of 0x2000, you can end up setting it to the end address of 0x2000, 
which could be less that the new address for the start of "func1". So lots of 
stuff goes wrong in the linking phase, especially when LTO is involved.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68655/new/

https://reviews.llvm.org/D68655

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

[Lldb-commits] [PATCH] D141318: [lldb] Store shared pointers in DieToTypePtr map instead of raw pointers

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 updated this revision to Diff 489137.
augusto2112 added a comment.

Make Type constructor private


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141318/new/

https://reviews.llvm.org/D141318

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolFileOnDemand.h
  lldb/include/lldb/Symbol/Type.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/unittests/Symbol/TestTypeSystemClang.cpp

Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp
===
--- lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -969,13 +969,10 @@
   ScratchTypeSystemClang::InferIsolatedASTKindFromLangOpts(lang_opts));
 }
 
-TEST_F(TestTypeSystemClang, GetExeModuleWhenMissingSymbolFile) {
-  CompilerType compiler_type = m_ast->GetBasicTypeFromAST(lldb::eBasicTypeInt);
-  lldb_private::Type t(0, nullptr, ConstString("MyType"), std::nullopt, nullptr,
-   0, {}, {}, compiler_type,
-   lldb_private::Type::ResolveState::Full);
-  // Test that getting the execution module when no type system is present
-  // is handled gracefully.
-  ModuleSP module = t.GetExeModule();
-  EXPECT_EQ(module.get(), nullptr);
+TEST_F(TestTypeSystemClang, GetDeclContextByNameWhenMissingSymbolFile) {
+  // Test that a type system without a symbol file is handled gracefully.
+  std::vector decls =
+  m_ast->DeclContextFindDeclByName(nullptr, ConstString("SomeName"), true);
+
+  EXPECT_TRUE(decls.empty());
 }
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -586,7 +586,6 @@
   lldb::TypeSP result = pdb->CreateLLDBTypeFromPDBType(*pdb_type);
   if (result) {
 m_types.insert(std::make_pair(type_uid, result));
-GetTypeList().Insert(result);
   }
   return result.get();
 }
Index: lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -465,10 +465,9 @@
   clang_type = clang_type.AddVolatileModifier();
 
 GetDeclarationForSymbol(type, decl);
-return std::make_shared(
-type.getSymIndexId(), m_ast.GetSymbolFile(), ConstString(name),
-udt->getLength(), nullptr, LLDB_INVALID_UID,
-lldb_private::Type::eEncodingIsUID, decl, clang_type,
+return m_ast.GetSymbolFile()->MakeType(
+type.getSymIndexId(), ConstString(name), udt->getLength(), nullptr,
+LLDB_INVALID_UID, lldb_private::Type::eEncodingIsUID, decl, clang_type,
 type_resolve_state);
   } break;
   case PDB_SymType::Enum: {
@@ -535,10 +534,10 @@
   ast_enum = ast_enum.AddVolatileModifier();
 
 GetDeclarationForSymbol(type, decl);
-return std::make_shared(
-type.getSymIndexId(), m_ast.GetSymbolFile(), ConstString(name), bytes,
-nullptr, LLDB_INVALID_UID, lldb_private::Type::eEncodingIsUID, decl,
-ast_enum, lldb_private::Type::ResolveState::Full);
+return m_ast.GetSymbolFile()->MakeType(
+type.getSymIndexId(), ConstString(name), bytes, nullptr,
+LLDB_INVALID_UID, lldb_private::Type::eEncodingIsUID, decl, ast_enum,
+lldb_private::Type::ResolveState::Full);
   } break;
   case PDB_SymType::Typedef: {
 auto type_def = llvm::dyn_cast(&type);
@@ -584,11 +583,10 @@
 std::optional size;
 if (type_def->getLength())
   size = type_def->getLength();
-return std::make_shared(
-type_def->getSymIndexId(), m_ast.GetSymbolFile(), ConstString(name),
-size, nullptr, target_type->GetID(),
-lldb_private::Type::eEncodingIsTypedefUID, decl, ast_typedef,
-lldb_private::Type::ResolveState::Full);
+return m_ast.GetSymbolFile()->MakeType(
+type_def->getSymIndexId(), ConstString(name), size, nullptr,
+target_type->GetID(), lldb_private::Type::eEncodingIsTypedefUID, decl,
+ast_typedef, lldb_private::Type::ResolveState::Full);
   } break;
   case PDB_SymType::Function:
   case PDB_SymType::FunctionSig: {
@@ -662,11 +660,10 @@
  arg_list.size(), is_variadic, type_quals, cc);
 
 GetDeclarationForSymbol(type, decl);
-return std::make_shared(
-type.getSymIndexId(), m_ast.GetSymbolFile(), ConstString(name),
-std::nullopt, nullptr, LLDB_INVALID_UID,
-lldb_private::Type::eEncodingIsUID, decl, func_sig_ast_type,
-lldb_private

[Lldb-commits] [PATCH] D141318: [lldb] Store shared pointers in DieToTypePtr map instead of raw pointers

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 added a comment.

@clayborg I went with your suggestion and made Type's constructor private, and 
any new Type created is automatically added to the TypeList. I think later on 
we should make TypeList keep a vector of unique pointers instead of shared ones.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141318/new/

https://reviews.llvm.org/D141318

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

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

Seems okay to me.
@JDevlieghere How does this look to you?




Comment at: 
lldb/examples/python/scripted_process/crashlog_scripted_process.py:34-37
+for section in image.section_infos:
+if section.start_addr and section.name == "__TEXT":
+self.loaded_images.append({"uuid": 
str(image.uuid),
+   "load_addr": 
section.start_addr})

mib wrote:
> bulbazord wrote:
> > I don't understand the intent of this part. It looks like you're changing 
> > the format of `self.loaded_images` here. It's still a List, but instead of 
> > containing images it contains specific information about specific sections 
> > of each image. If the format has changed, don't consumers of 
> > `get_loaded_images` need to be modified as well?
> The list for this specific scripted process class was wrong, it expects the 
> current format. Previously we didn't make use of `loaded_images` for the 
> crashlog_scripted_process class, we just relied on some ad hoc heuristic to 
> load the modules. 
> The other consumers of `get_loaded_images` are fine.
Ah, that makes sense. If one wanted to know what the format of `loaded_images` 
*should* be, how would they find out? Is that information documented or encoded 
anywhere? Or is it more ad-hoc?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141702/new/

https://reviews.llvm.org/D141702

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

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

[Med Ismail Bennani via Phabricator via lldb-commits]

mib marked an inline comment as done.
mib added inline comments.



Comment at: 
lldb/examples/python/scripted_process/crashlog_scripted_process.py:34-37
+for section in image.section_infos:
+if section.start_addr and section.name == "__TEXT":
+self.loaded_images.append({"uuid": 
str(image.uuid),
+   "load_addr": 
section.start_addr})

bulbazord wrote:
> mib wrote:
> > bulbazord wrote:
> > > I don't understand the intent of this part. It looks like you're changing 
> > > the format of `self.loaded_images` here. It's still a List, but instead 
> > > of containing images it contains specific information about specific 
> > > sections of each image. If the format has changed, don't consumers of 
> > > `get_loaded_images` need to be modified as well?
> > The list for this specific scripted process class was wrong, it expects the 
> > current format. Previously we didn't make use of `loaded_images` for the 
> > crashlog_scripted_process class, we just relied on some ad hoc heuristic to 
> > load the modules. 
> > The other consumers of `get_loaded_images` are fine.
> Ah, that makes sense. If one wanted to know what the format of 
> `loaded_images` *should* be, how would they find out? Is that information 
> documented or encoded anywhere? Or is it more ad-hoc?
It's documented in the `get_loaded_images` in the base `ScriptedProcess` python 
class.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141702/new/

https://reviews.llvm.org/D141702

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

[Lldb-commits] [PATCH] D141318: [lldb] Store shared pointers in DieToTypePtr map instead of raw pointers

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

New changes looks great, thanks for tackling this. Just a few comments that 
need to be updated and this will be good to go as long as the test suite passes 
just fine!

In D141318#4052934 , @augusto2112 
wrote:

> @clayborg I went with your suggestion and made Type's constructor private, 
> and any new Type created is automatically added to the TypeList. I think 
> later on we should make TypeList keep a vector of unique pointers instead of 
> shared ones.

If we did switch the unique pointers, then we would always need to hand out 
"Type *" and this could cause crashes and use after free errors. So I would 
hold off on that change.




Comment at: lldb/include/lldb/Symbol/SymbolFile.h:414
   virtual void SetDebugInfoHadFrameVariableErrors() = 0;
 
+  virtual lldb::TypeSP

Just add a comment here saying something lie "This function is used to create 
types that belong to a SymbolFile. The symbol file will own a strong reference 
to the type in an internal type list."



Comment at: lldb/include/lldb/Symbol/Type.h:229-230
+private:
+  /// Only allow Symbol File to create types, as they should own them by 
keeping
+  /// them in their TypeList.
+  friend class lldb_private::SymbolFileCommon;

add a \see SymbolFileCommon::MakeType() reference in the header documentation 
here so users will know what function to use if the get a compile error.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141318/new/

https://reviews.llvm.org/D141318

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

[Lldb-commits] [PATCH] D136928: [LLDB] Fix help text for "platform settings"

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Sorry for the delay!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136928/new/

https://reviews.llvm.org/D136928

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

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:453
+Status error;
+if (!Symbols::DownloadObjectAndSymbolFile(module_spec, error, true) ||
+error.Fail() ||

The last argument here is `force_lookup` (you should probably add that as an 
inline comment). This means that for any scripted process (not just interactive 
crashlogs) we're going to do a dSYMForUUID call (like we do for loading a 
kernel). I suppose that's what you want for interactive crashlogs, but not for 
a generic scripted process implementation. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141702/new/

https://reviews.llvm.org/D141702

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

[Lldb-commits] [PATCH] D141658: [lldb/crashlog] Make interactive mode the new default

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 489151.
mib marked an inline comment as not done.
mib added a comment.

Address @JDevlieghere comment:

- Check is stdout is a tty to run in interactive mode
- Remove extra return line


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141658/new/

https://reviews.llvm.org/D141658

Files:
  lldb/examples/python/crashlog.py
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive_crashlog_invalid_target.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test

Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
@@ -1,7 +1,7 @@
 # RUN: %clang_host -g %S/Inputs/test.c -o %t.out
 # RUN: cp %S/Inputs/a.out.crash %t.crash
 # RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":20, "bar":9, "foo":16}'
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog -b %t.crash' 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
 
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
@@ -3,7 +3,7 @@
 # RUN: mkdir -p %t.dir
 # RUN: yaml2obj %S/Inputs/interactive_crashlog/multithread-test.yaml > %t.dir/multithread-test
 # RUN: %lldb -b -o 'command script import lldb.macosx.crashlog' \
-# RUN: -o 'crashlog -a -i -s -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
+# RUN: -o 'crashlog -a -s -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
 # RUN: -o 'command source -s 0 %s' 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
@@ -3,7 +3,7 @@
 # RUN: mkdir -p %t.dir
 # RUN: yaml2obj %S/Inputs/interactive_crashlog/multithread-test.yaml > %t.dir/multithread-test
 # RUN: %lldb -o 'command script import lldb.macosx.crashlog' \
-# RUN: -o 'crashlog -a -i -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
+# RUN: -o 'crashlog -a -t %t.dir/multithread-test %S/Inputs/interactive_crashlog/multithread-test.ips' \
 # RUN: -o "thread list" -o "bt all" 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
@@ -2,7 +2,7 @@
 
 # RUN: cp %S/Inputs/no_threadState.ips %t.crash
 # RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":20, "bar":9, "foo":16}' --json
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog -b %t.crash' 2>&1 | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
 
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
@@ -2,11 +2,11 @@
 
 # RUN: cp %S/Inputs/a.out.ips %t.crash
 # RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":20, "bar":9, "foo":16}' --json
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macos

[Lldb-commits] [PATCH] D141658: [lldb/crashlog] Make interactive mode the new default

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM. You can remove one of the `-b` in the test to check the TTY stuff (it 
should be false when piping into FileCheck).




Comment at: lldb/examples/python/crashlog.py:1302
+print("Aborting symbolication.")
+print()
+option_parser.print_help()

mib wrote:
> JDevlieghere wrote:
> > Spurious print? It doesn't look like we're printing a double newline 
> > elsewhere.
> I wanted to leave space between the actual error message and the help
I like this


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141658/new/

https://reviews.llvm.org/D141658

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

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 489152.
mib added a comment.

Only force symbol lookup for crashlog scripted process.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141702/new/

https://reviews.llvm.org/D141702

Files:
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -11,7 +11,7 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
-
+#include "lldb/Core/Progress.h"
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
@@ -19,6 +19,7 @@
 #include "lldb/Interpreter/OptionGroupBoolean.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Interpreter/ScriptedMetadata.h"
+#include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/Queue.h"
 #include "lldb/Target/RegisterContext.h"
@@ -410,15 +411,19 @@
 
   StructuredData::ArraySP loaded_images_sp = GetInterface().GetLoadedImages();
 
-  if (!loaded_images_sp || !loaded_images_sp->GetSize())
+  size_t num_image_to_load = loaded_images_sp->GetSize();
+  if (!loaded_images_sp || !num_image_to_load)
 return ScriptedInterface::ErrorWithMessage(
 LLVM_PRETTY_FUNCTION, "No loaded images.", error);
 
   ModuleList module_list;
   Target &target = GetTarget();
+  bool force_lookup = m_scripted_metadata.GetClassName().contains("CrashLog");
 
-  auto reload_image = [&target, &module_list, &error_with_message](
-  StructuredData::Object *obj) -> bool {
+  Progress progress("Fetching external dependencies", num_image_to_load);
+  auto reload_image = [&target, &module_list, &error_with_message, &progress,
+   &force_lookup](StructuredData::Object *obj) -> bool {
+progress.Increment();
 StructuredData::Dictionary *dict = obj->GetAsDictionary();
 
 if (!dict)
@@ -445,6 +450,13 @@
 }
 module_spec.GetArchitecture() = target.GetArchitecture();
 
+Status error;
+if (!Symbols::DownloadObjectAndSymbolFile(module_spec, error,
+  force_lookup) ||
+error.Fail() ||
+!FileSystem::Instance().Exists(module_spec.GetFileSpec()))
+  return error_with_message(error.AsCString());
+
 ModuleSP module_sp =
 target.GetOrCreateModule(module_spec, true /* notify */);
 
@@ -469,9 +481,11 @@
 if (!changed && !module_sp->GetObjectFile())
   return error_with_message("Couldn't set the load address for module.");
 
-dict->GetValueForKeyAsString("path", value);
-FileSpec objfile(value);
-module_sp->SetFileSpecAndObjectName(objfile, objfile.GetFilename());
+if (has_path) {
+  dict->GetValueForKeyAsString("path", value);
+  FileSpec objfile(value);
+  module_sp->SetFileSpecAndObjectName(objfile, objfile.GetFilename());
+}
 
 return module_list.AppendIfNeeded(module_sp);
   };
Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -31,12 +31,10 @@
 if image not in self.loaded_images:
 if image.uuid == uuid.UUID(int=0):
 continue
-err = image.add_module(self.target)
-if err:
-# Append to SBCommandReturnObject
-print(err)
-else:
-self.loaded_images.append(image)
+for section in image.section_infos:
+if section.start_addr and section.name == "__TEXT":
+self.loaded_images.append({"uuid": 
str(image.uuid),
+   "load_addr": 
section.start_addr})
 
 for thread in crash_log.threads:
 if self.load_all_images:


Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -11,7 +11,7 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
-
+#include "lldb/Core/Progress.h"
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
@@ -19,6 +19,7 @@
 #include "lldb/Interpreter/Opt

[Lldb-commits] [lldb] b1f4f06 - [LLDB][LoongArch] Delete the s9 register alias definition

[Weining Lu via lldb-commits]

Author: Hui Li
Date: 2023-01-14T09:22:14+08:00
New Revision: b1f4f06dede54627c546d0e7f21e7e72295f280b

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

LOG: [LLDB][LoongArch] Delete the s9 register alias definition

In RegisterInfos_loongarch64.h, r22 is defined twice. Having an extra array
member causes problems reading and writing registers defined after r22. So,
for r22, keep the alias fp, delete the s9 alias.

The PC register is incorrectly accessed when the step command is executed.
The step command behavior is incorrect.

This test reflects this problem:

```
loongson@linux:~$ cat test.c

 #include 

int func(int a) {
  return a + 1;
}

int main(int argc, char const *argv[]) {
  func(10);
  return 0;
}

loongson@linux:~$ clang -g test.c  -o test

```

Without this patch:
```
loongson@linux:~$ llvm-project/llvm/build/bin/lldb test
(lldb) target create "test"
Current executable set to '/home/loongson/test' (loongarch64).
(lldb) b main
Breakpoint 1: where = test`main + 40 at test.c:8:3, address = 0x00012668
(lldb) r
Process 278049 launched: '/home/loongson/test' (loongarch64)
Process 278049 stopped
* thread #1, name = 'test', stop reason = breakpoint 1.1
frame #0: 0x00012668 test`main(argc=1, argv=0x7fff72a8) at 
test.c:8:3
   5}
   6
   7int main(int argc, char const *argv[]) {
-> 8  func(10);
   9  return 0;
   10   }
   11
(lldb) s
Process 278049 stopped
* thread #1, name = 'test', stop reason = step in
frame #0: 0x00012670 test`main(argc=1, argv=0x7fff72a8) at 
test.c:9:3
   6
   7int main(int argc, char const *argv[]) {
   8  func(10);
-> 9  return 0;
   10   }

```

With this patch:

```
loongson@linux:~$ llvm-project/llvm/build/bin/lldb test
(lldb) target create "test"
Current executable set to '/home/loongson/test' (loongarch64).
(lldb) b main
Breakpoint 1: where = test`main + 40 at test.c:8:3, address = 0x00012668
(lldb) r
Process 278632 launched: '/home/loongson/test' (loongarch64)
Process 278632 stopped
* thread #1, name = 'test', stop reason = breakpoint 1.1
frame #0: 0x00012668 test`main(argc=1, argv=0x7fff72a8) at 
test.c:8:3
   5}
   6
   7int main(int argc, char const *argv[]) {
-> 8  func(10);
   9  return 0;
   10   }
   11
(lldb) s
Process 278632 stopped
* thread #1, name = 'test', stop reason = step in
frame #0: 0x00012624 test`func(a=10) at test.c:4:10
   1 #include 
   2
   3int func(int a) {
-> 4  return a + 1;
   5}

```

Reviewed By: SixWeining, DavidSpickett

Differential Revision: https://reviews.llvm.org/D140615

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/RegisterInfos_loongarch64.h
lldb/source/Plugins/Process/Utility/lldb-loongarch-register-enums.h
lldb/source/Utility/LoongArch_DWARF_Registers.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/RegisterInfos_loongarch64.h 
b/lldb/source/Plugins/Process/Utility/RegisterInfos_loongarch64.h
index 723881bb92750..27f2bac22dd51 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfos_loongarch64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfos_loongarch64.h
@@ -100,7 +100,6 @@ static lldb_private::RegisterInfo 
g_register_infos_loongarch64[] = {
 DEFINE_GPR64_ALT(r20, t8, LLDB_INVALID_REGNUM),
 DEFINE_GPR64(r21, LLDB_INVALID_REGNUM),
 DEFINE_GPR64_ALT(r22, fp, LLDB_REGNUM_GENERIC_FP),
-DEFINE_GPR64_ALT(r22, s9, LLDB_REGNUM_GENERIC_FP),
 DEFINE_GPR64_ALT(r23, s0, LLDB_INVALID_REGNUM),
 DEFINE_GPR64_ALT(r24, s1, LLDB_INVALID_REGNUM),
 DEFINE_GPR64_ALT(r25, s2, LLDB_INVALID_REGNUM),

diff  --git 
a/lldb/source/Plugins/Process/Utility/lldb-loongarch-register-enums.h 
b/lldb/source/Plugins/Process/Utility/lldb-loongarch-register-enums.h
index d53e8ce1b3f7d..f55c807f86c00 100644
--- a/lldb/source/Plugins/Process/Utility/lldb-loongarch-register-enums.h
+++ b/lldb/source/Plugins/Process/Utility/lldb-loongarch-register-enums.h
@@ -85,7 +85,6 @@ enum {
   gpr_t7_loongarch = gpr_r19_loongarch,
   gpr_t8_loongarch = gpr_r20_loongarch,
   gpr_fp_loongarch = gpr_r22_loongarch,
-  gpr_s9_loongarch = gpr_r22_loongarch,
   gpr_s0_loongarch = gpr_r23_loongarch,
   gpr_s1_loongarch = gpr_r24_loongarch,
   gpr_s2_loongarch = gpr_r25_loongarch,

diff  --git a/lldb/source/Utility/LoongArch_DWARF_Registers.h 
b/lldb/source/Utility/LoongArch_DWARF_Registers.h
index b9c1928531442..34e40a066051e 100644
--- a/lldb/source/Utility/LoongArch_DWARF_Registers.h
+++ b/lldb/source/Utility/LoongArch_DWARF_Registers.h
@@ -128,7 +128,6 @@ enum {
   dwarf_gpr_t7 = dwarf_gpr_r19,
   dwarf_gpr_t8 = dwarf_gpr_r20,
   dwarf_gpr_fp = dwarf_gpr_r22,
-  dwarf_gpr_s9 = dwarf_gpr_r22,
   dwarf_gpr_s0 = dwarf_gpr_r23,
   dwar

[Lldb-commits] [PATCH] D140615: [LLDB][LoongArch] Delete the s9 register alias definition

[Lu Weining via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb1f4f06dede5: [LLDB][LoongArch] Delete the s9 register alias 
definition (authored by lh03061238, committed by SixWeining).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140615/new/

https://reviews.llvm.org/D140615

Files:
  lldb/source/Plugins/Process/Utility/RegisterInfos_loongarch64.h
  lldb/source/Plugins/Process/Utility/lldb-loongarch-register-enums.h
  lldb/source/Utility/LoongArch_DWARF_Registers.h


Index: lldb/source/Utility/LoongArch_DWARF_Registers.h
===
--- lldb/source/Utility/LoongArch_DWARF_Registers.h
+++ lldb/source/Utility/LoongArch_DWARF_Registers.h
@@ -128,7 +128,6 @@
   dwarf_gpr_t7 = dwarf_gpr_r19,
   dwarf_gpr_t8 = dwarf_gpr_r20,
   dwarf_gpr_fp = dwarf_gpr_r22,
-  dwarf_gpr_s9 = dwarf_gpr_r22,
   dwarf_gpr_s0 = dwarf_gpr_r23,
   dwarf_gpr_s1 = dwarf_gpr_r24,
   dwarf_gpr_s2 = dwarf_gpr_r25,
Index: lldb/source/Plugins/Process/Utility/lldb-loongarch-register-enums.h
===
--- lldb/source/Plugins/Process/Utility/lldb-loongarch-register-enums.h
+++ lldb/source/Plugins/Process/Utility/lldb-loongarch-register-enums.h
@@ -85,7 +85,6 @@
   gpr_t7_loongarch = gpr_r19_loongarch,
   gpr_t8_loongarch = gpr_r20_loongarch,
   gpr_fp_loongarch = gpr_r22_loongarch,
-  gpr_s9_loongarch = gpr_r22_loongarch,
   gpr_s0_loongarch = gpr_r23_loongarch,
   gpr_s1_loongarch = gpr_r24_loongarch,
   gpr_s2_loongarch = gpr_r25_loongarch,
Index: lldb/source/Plugins/Process/Utility/RegisterInfos_loongarch64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_loongarch64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_loongarch64.h
@@ -100,7 +100,6 @@
 DEFINE_GPR64_ALT(r20, t8, LLDB_INVALID_REGNUM),
 DEFINE_GPR64(r21, LLDB_INVALID_REGNUM),
 DEFINE_GPR64_ALT(r22, fp, LLDB_REGNUM_GENERIC_FP),
-DEFINE_GPR64_ALT(r22, s9, LLDB_REGNUM_GENERIC_FP),
 DEFINE_GPR64_ALT(r23, s0, LLDB_INVALID_REGNUM),
 DEFINE_GPR64_ALT(r24, s1, LLDB_INVALID_REGNUM),
 DEFINE_GPR64_ALT(r25, s2, LLDB_INVALID_REGNUM),


Index: lldb/source/Utility/LoongArch_DWARF_Registers.h
===
--- lldb/source/Utility/LoongArch_DWARF_Registers.h
+++ lldb/source/Utility/LoongArch_DWARF_Registers.h
@@ -128,7 +128,6 @@
   dwarf_gpr_t7 = dwarf_gpr_r19,
   dwarf_gpr_t8 = dwarf_gpr_r20,
   dwarf_gpr_fp = dwarf_gpr_r22,
-  dwarf_gpr_s9 = dwarf_gpr_r22,
   dwarf_gpr_s0 = dwarf_gpr_r23,
   dwarf_gpr_s1 = dwarf_gpr_r24,
   dwarf_gpr_s2 = dwarf_gpr_r25,
Index: lldb/source/Plugins/Process/Utility/lldb-loongarch-register-enums.h
===
--- lldb/source/Plugins/Process/Utility/lldb-loongarch-register-enums.h
+++ lldb/source/Plugins/Process/Utility/lldb-loongarch-register-enums.h
@@ -85,7 +85,6 @@
   gpr_t7_loongarch = gpr_r19_loongarch,
   gpr_t8_loongarch = gpr_r20_loongarch,
   gpr_fp_loongarch = gpr_r22_loongarch,
-  gpr_s9_loongarch = gpr_r22_loongarch,
   gpr_s0_loongarch = gpr_r23_loongarch,
   gpr_s1_loongarch = gpr_r24_loongarch,
   gpr_s2_loongarch = gpr_r25_loongarch,
Index: lldb/source/Plugins/Process/Utility/RegisterInfos_loongarch64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_loongarch64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_loongarch64.h
@@ -100,7 +100,6 @@
 DEFINE_GPR64_ALT(r20, t8, LLDB_INVALID_REGNUM),
 DEFINE_GPR64(r21, LLDB_INVALID_REGNUM),
 DEFINE_GPR64_ALT(r22, fp, LLDB_REGNUM_GENERIC_FP),
-DEFINE_GPR64_ALT(r22, s9, LLDB_REGNUM_GENERIC_FP),
 DEFINE_GPR64_ALT(r23, s0, LLDB_INVALID_REGNUM),
 DEFINE_GPR64_ALT(r24, s1, LLDB_INVALID_REGNUM),
 DEFINE_GPR64_ALT(r25, s2, LLDB_INVALID_REGNUM),
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] b7ae576 - [LLDB][LoongArch] Add FP branch instructions for EmulateInstructionLoongArch

[Weining Lu via lldb-commits]

Author: Hui Li
Date: 2023-01-14T09:22:18+08:00
New Revision: b7ae5762a110ee7c28b218b4ed4e3e24b2b3c64d

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

LOG: [LLDB][LoongArch] Add FP branch instructions for 
EmulateInstructionLoongArch

Add floating-point branch Instructions for EmulateInstructionLoongArch and
add relevant unit tests.

Without this patch:

```
$ ninja check-lldb-unit
[0/1] Running lldb unit test suite

Testing Time: 10.45s
  Passed: 1044
```

With this patch:

```
$ ninja check-lldb-unit
[0/1] Running lldb unit test suite

Testing Time: 10.20s
  Passed: 1048

```

Reviewed By: SixWeining, MaskRay, DavidSpickett

Differential Revision: https://reviews.llvm.org/D140759

Added: 


Modified: 
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
lldb/unittests/Instruction/LoongArch/TestLoongArchEmulator.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp 
b/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
index c17da8b25aa31..adfd57dff07c0 100644
--- a/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
+++ b/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
@@ -40,6 +40,10 @@ 
EmulateInstructionLoongArch::GetOpcodeForInstruction(uint32_t inst) {
"beqz rj, offs21"},
   {0xfc00, 0x4400, &EmulateInstructionLoongArch::EmulateBNEZ,
"bnez rj, offs21"},
+  {0xfc000300, 0x4800, &EmulateInstructionLoongArch::EmulateBCEQZ,
+   "bceqz cj, offs21"},
+  {0xfc000300, 0x48000100, &EmulateInstructionLoongArch::EmulateBCNEZ,
+   "bcnez cj, offs21"},
   {0xfc00, 0x4c00, &EmulateInstructionLoongArch::EmulateJIRL,
"jirl rd, rj, offs16"},
   {0xfc00, 0x5000, &EmulateInstructionLoongArch::EmulateB,
@@ -217,6 +221,14 @@ bool EmulateInstructionLoongArch::EmulateBNEZ(uint32_t 
inst) {
   return IsLoongArch64() ? EmulateBNEZ64(inst) : false;
 }
 
+bool EmulateInstructionLoongArch::EmulateBCEQZ(uint32_t inst) {
+  return IsLoongArch64() ? EmulateBCEQZ64(inst) : false;
+}
+
+bool EmulateInstructionLoongArch::EmulateBCNEZ(uint32_t inst) {
+  return IsLoongArch64() ? EmulateBCNEZ64(inst) : false;
+}
+
 bool EmulateInstructionLoongArch::EmulateJIRL(uint32_t inst) {
   return IsLoongArch64() ? EmulateJIRL64(inst) : false;
 }
@@ -295,6 +307,50 @@ bool EmulateInstructionLoongArch::EmulateBNEZ64(uint32_t 
inst) {
 return WritePC(pc + 4);
 }
 
+// bceqz cj, offs21
+// if CFR[cj] == 0:
+// PC = PC + SignExtend({offs21, 2'b0}, GRLEN)
+bool EmulateInstructionLoongArch::EmulateBCEQZ64(uint32_t inst) {
+  bool success = false;
+  uint32_t cj = Bits32(inst, 7, 5) + fpr_fcc0_loongarch;
+  uint64_t pc = ReadPC(&success);
+  if (!success)
+return false;
+  uint32_t offs21 = Bits32(inst, 25, 10) + (Bits32(inst, 4, 0) << 16);
+  uint8_t cj_val =
+  (uint8_t)ReadRegisterUnsigned(eRegisterKindLLDB, cj, 0, &success);
+  if (!success)
+return false;
+  if (cj_val == 0) {
+uint64_t next_pc = pc + llvm::SignExtend64<23>(offs21 << 2);
+return WritePC(next_pc);
+  } else
+return WritePC(pc + 4);
+  return false;
+}
+
+// bcnez cj, offs21
+// if CFR[cj] != 0:
+// PC = PC + SignExtend({offs21, 2'b0}, GRLEN)
+bool EmulateInstructionLoongArch::EmulateBCNEZ64(uint32_t inst) {
+  bool success = false;
+  uint32_t cj = Bits32(inst, 7, 5) + fpr_fcc0_loongarch;
+  uint64_t pc = ReadPC(&success);
+  if (!success)
+return false;
+  uint32_t offs21 = Bits32(inst, 25, 10) + (Bits32(inst, 4, 0) << 16);
+  uint8_t cj_val =
+  (uint8_t)ReadRegisterUnsigned(eRegisterKindLLDB, cj, 0, &success);
+  if (!success)
+return false;
+  if (cj_val != 0) {
+uint64_t next_pc = pc + llvm::SignExtend64<23>(offs21 << 2);
+return WritePC(next_pc);
+  } else
+return WritePC(pc + 4);
+  return false;
+}
+
 // jirl rd, rj, offs16
 // GR[rd] = PC + 4
 // PC = GR[rj] + SignExtend({offs16, 2'b0}, GRLEN)

diff  --git 
a/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h 
b/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
index 15da499261b05..e03356244b476 100644
--- a/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
+++ b/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
@@ -75,6 +75,8 @@ class EmulateInstructionLoongArch : public EmulateInstruction 
{
 
   bool EmulateBEQZ(uint32_t inst);
   bool EmulateBNEZ(uint32_t inst);
+  bool EmulateBCEQZ(uint32_t inst);
+  bool EmulateBCNEZ(uint32_t inst);
   bool EmulateJIRL(uint32_t inst);
   bool EmulateB(uint32_t inst);
   bool EmulateBL(uint32_t inst);
@@ -88,6 +9

[Lldb-commits] [PATCH] D140759: [LLDB][LoongArch] Add FP branch instructions for EmulateInstructionLoongArch

[Lu Weining via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb7ae5762a110: [LLDB][LoongArch] Add FP branch instructions 
for EmulateInstructionLoongArch (authored by lh03061238, committed by 
SixWeining).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140759/new/

https://reviews.llvm.org/D140759

Files:
  lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
  lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
  lldb/unittests/Instruction/LoongArch/TestLoongArchEmulator.cpp

Index: lldb/unittests/Instruction/LoongArch/TestLoongArchEmulator.cpp
===
--- lldb/unittests/Instruction/LoongArch/TestLoongArchEmulator.cpp
+++ lldb/unittests/Instruction/LoongArch/TestLoongArchEmulator.cpp
@@ -38,6 +38,14 @@
 testBZcondBranch(this, name, false, rj_val_continued); \
   }
 
+#define GEN_BCZCOND_TEST(bit, name, cj_val_branched, cj_val_continued) \
+  TEST_F(LoongArch##bit##EmulatorTester, test##name##branched) {   \
+testBCZcondBranch(this, name, true, cj_val_branched);  \
+  }\
+  TEST_F(LoongArch##bit##EmulatorTester, test##name##continued) {  \
+testBCZcondBranch(this, name, false, cj_val_continued);\
+  }
+
 struct LoongArch64EmulatorTester : public EmulateInstructionLoongArch,
testing::Test {
   RegisterInfoPOSIX_loongarch64::GPR gpr;
@@ -136,8 +144,26 @@
   return EncodeBZcondType(0b010001, rj, uint32_t(offs21));
 }
 
+// BCEQZ BCNEZ
+static uint32_t EncodeBCZcondType(uint32_t opcode, uint8_t cj,
+  uint32_t offs21) {
+  uint32_t offs20_16 = (offs21 & 0x001f) >> 16;
+  uint32_t offs15_0 = offs21 & 0x;
+  return (opcode >> 2) << 26 | offs15_0 << 10 | (opcode & 0b11) << 8 | cj << 5 |
+ offs20_16;
+}
+
+static uint32_t BCEQZ(uint8_t cj, int32_t offs21) {
+  return EncodeBCZcondType(0b01001000, cj, uint32_t(offs21));
+}
+
+static uint32_t BCNEZ(uint8_t cj, int32_t offs21) {
+  return EncodeBCZcondType(0b01001001, cj, uint32_t(offs21));
+}
+
 using EncoderBcond = uint32_t (*)(uint32_t rj, uint32_t rd, int32_t offs16);
 using EncoderBZcond = uint32_t (*)(uint32_t rj, int32_t offs21);
+using EncoderBCZcond = uint32_t (*)(uint8_t cj, int32_t offs21);
 
 TEST_F(LoongArch64EmulatorTester, testJIRL) {
   bool success = false;
@@ -220,6 +246,21 @@
   ASSERT_EQ(pc, old_pc + (branched ? (-256 * 4) : 4));
 }
 
+static void testBCZcondBranch(LoongArch64EmulatorTester *tester,
+  EncoderBCZcond encoder, bool branched,
+  uint32_t cj_val) {
+  bool success = false;
+  addr_t old_pc = 0x12000600;
+  tester->WritePC(old_pc);
+  tester->fpr.fcc = cj_val;
+  // bcz fcc0, 256
+  uint32_t inst = encoder(0, 256);
+  ASSERT_TRUE(tester->TestExecute(inst));
+  auto pc = tester->ReadPC(&success);
+  ASSERT_TRUE(success);
+  ASSERT_EQ(pc, old_pc + (branched ? (256 * 4) : 4));
+}
+
 GEN_BCOND_TEST(64, BEQ, 1, 1, 0)
 GEN_BCOND_TEST(64, BNE, 1, 0, 1)
 GEN_BCOND_TEST(64, BLT, -2, 1, -3)
@@ -228,3 +269,5 @@
 GEN_BCOND_TEST(64, BGEU, -2, 1, -1)
 GEN_BZCOND_TEST(64, BEQZ, 0, 1)
 GEN_BZCOND_TEST(64, BNEZ, 1, 0)
+GEN_BCZCOND_TEST(64, BCEQZ, 0, 1)
+GEN_BCZCOND_TEST(64, BCNEZ, 1, 0)
Index: lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
===
--- lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
+++ lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
@@ -75,6 +75,8 @@
 
   bool EmulateBEQZ(uint32_t inst);
   bool EmulateBNEZ(uint32_t inst);
+  bool EmulateBCEQZ(uint32_t inst);
+  bool EmulateBCNEZ(uint32_t inst);
   bool EmulateJIRL(uint32_t inst);
   bool EmulateB(uint32_t inst);
   bool EmulateBL(uint32_t inst);
@@ -88,6 +90,8 @@
 
   bool EmulateBEQZ64(uint32_t inst);
   bool EmulateBNEZ64(uint32_t inst);
+  bool EmulateBCEQZ64(uint32_t inst);
+  bool EmulateBCNEZ64(uint32_t inst);
   bool EmulateJIRL64(uint32_t inst);
   bool EmulateB64(uint32_t inst);
   bool EmulateBL64(uint32_t inst);
Index: lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
===
--- lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
+++ lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
@@ -40,6 +40,10 @@
"beqz rj, offs21"},
   {0xfc00, 0x4400, &EmulateInstructionLoongArch::EmulateBNEZ,
"bnez rj, offs21"},
+  {0xfc000300, 0x4800, &EmulateInstructionLoongArch::EmulateBCEQZ,
+   "bceqz cj, offs21"},
+  {0xfc000300, 0x48000100, &EmulateInstructionLoongArch::EmulateBCNEZ,
+   "bcne