date:20210521

[Lldb-commits] [PATCH] D102208: Remove Windows editline from LLDB

2021-05-21 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

> Thanks, all. If there are no more comments, could someone land it for me? I 
> don't have commit access.

Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102208

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

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

2021-05-21 Thread Fangrui Song via Phabricator via lldb-commits

MaskRay added a comment.

Can `createMCObjectFileInfo` return `MCObjectFileInfo` instead of 
`std::unique_ptr`?




Comment at: clang/lib/Parse/ParseStmtAsm.cpp:590
+
   if (!MAI || !MII || !MOFI || !STI) {
 Diag(AsmLoc, diag::err_msasm_unable_to_create_target)

Can `MOFI` be null? (i.e. can createMCObjectFileInfo guarantee no-null return 
value?)

Consider moving the construction below the checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101921

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

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

2021-05-21 Thread Philipp Krones via Phabricator via lldb-commits

flip1995 updated this revision to Diff 344849.
flip1995 added a comment.
Herald added subscribers: lldb-commits, atanasyan, jrtc27.
Herald added a project: LLDB.

rebased and addressed review comments:

- [MC] Remove MOFI argument from MCContext constructor
- [MC] Remove getTextSectionAlignment function from MCObjectFileInfo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101921

Files:
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/tools/driver/cc1as_main.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
  llvm/include/llvm/MC/MCContext.h
  llvm/include/llvm/Support/TargetRegistry.h
  llvm/lib/CodeGen/MachineModuleInfo.cpp
  llvm/lib/DWARFLinker/DWARFStreamer.cpp
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCDisassembler/Disassembler.cpp
  llvm/lib/Object/ModuleSymbolTable.cpp
  llvm/tools/llvm-cfi-verify/lib/FileAnalysis.cpp
  llvm/tools/llvm-cfi-verify/lib/FileAnalysis.h
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-exegesis/lib/Analysis.cpp
  llvm/tools/llvm-exegesis/lib/Analysis.h
  llvm/tools/llvm-exegesis/lib/LlvmState.cpp
  llvm/tools/llvm-exegesis/lib/SnippetFile.cpp
  llvm/tools/llvm-jitlink/llvm-jitlink.cpp
  llvm/tools/llvm-mc-assemble-fuzzer/llvm-mc-assemble-fuzzer.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-ml/Disassembler.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-objdump/MachODump.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-profgen/ProfiledBinary.cpp
  llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
  llvm/tools/sancov/sancov.cpp
  llvm/unittests/CodeGen/MachineInstrTest.cpp
  llvm/unittests/CodeGen/MachineOperandTest.cpp
  llvm/unittests/CodeGen/TestAsmPrinter.cpp
  llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
  llvm/unittests/MC/DwarfLineTables.cpp
  llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
  mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp

Index: mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
===
--- mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
+++ mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
@@ -168,9 +168,10 @@
   target->createMCAsmInfo(*mri, this->triple, mcOptions));
   mai->setRelaxELFRelocations(true);
 
-  llvm::MCObjectFileInfo mofi;
-  llvm::MCContext ctx(triple, mai.get(), mri.get(), &mofi, &srcMgr, &mcOptions);
-  mofi.initMCObjectFileInfo(ctx, /*PIC=*/false, /*LargeCodeModel=*/false);
+  llvm::MCContext ctx(triple, mai.get(), mri.get(), &srcMgr, &mcOptions);
+  std::unique_ptr mofi(target->createMCObjectFileInfo(
+  ctx, /*PIC=*/false, /*LargeCodeModel=*/false));
+  ctx.setObjectFileInfo(mofi.get());
 
   SmallString<128> cwd;
   if (!llvm::sys::fs::current_path(cwd))
Index: llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
===
--- llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
+++ llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
@@ -62,6 +62,7 @@
   std::unique_ptr MRI;
   std::unique_ptr MUPMAI;
   std::unique_ptr MII;
+  std::unique_ptr MOFI;
   std::unique_ptr Str;
   std::unique_ptr Parser;
   std::unique_ptr Ctx;
@@ -74,7 +75,6 @@
   const Target *TheTarget;
 
   const MCTargetOptions MCOptions;
-  MCObjectFileInfo MOFI;
 
   SystemZAsmLexerTest() {
 // We will use the SystemZ triple, because of missing
@@ -112,9 +112,11 @@
 SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
 EXPECT_EQ(Buffer, nullptr);
 
-Ctx.reset(new MCContext(Triple, MUPMAI.get(), MRI.get(), &MOFI, STI.get(),
-&SrcMgr, &MCOptions));
-MOFI.initMCObjectFileInfo(*Ctx, /*PIC=*/false, /*LargeCodeModel=*/false);
+Ctx.reset(new MCContext(Triple, MUPMAI.get(), MRI.get(), STI.get(), &SrcMgr,
+&MCOptions));
+MOFI.reset(TheTarget->createMCObjectFileInfo(*Ctx, /*PIC=*/false,
+ /*LargeCodeModel=*/false));
+Ctx->setObjectFileInfo(MOFI.get());
 
 Str.reset(TheTarget->createNullStreamer(*Ctx));
 
Index: llvm/unittests/MC/DwarfLineTables.cpp
===
--- llvm/unittests/MC/DwarfLineTables.cpp
+++ llvm/unittests/MC/DwarfLineTables.cpp
@@ -41,7 +41,7 @@
 MCTargetOptions MCOptions;
 MAI.reset(TheTarget->createMCAsmInfo(*MRI, TripleName, MCOptions));
 Ctx = std::make_unique(Triple(TripleName), MAI.get(), MRI.get(),
-  /*MOFI=*/nullptr, /*MSTI=*/nullptr);
+  /*MSTI=*/nullptr);
   }
 
   operator bool() { return Ctx.get(); }
Index: llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
===
--- llvm/un

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

2021-05-21 Thread Philipp Krones via Phabricator via lldb-commits

flip1995 added a comment.

In D101921#2754426 , @MaskRay wrote:

> Can `createMCObjectFileInfo` return `MCObjectFileInfo` instead of 
> `std::unique_ptr`?

`createMCObjectfileInfo` returns a `MCObjectFileInfo *` similar to every other 
`create*` function in `TargetRegistry.h`.




Comment at: clang/lib/Parse/ParseStmtAsm.cpp:590
+
   if (!MAI || !MII || !MOFI || !STI) {
 Diag(AsmLoc, diag::err_msasm_unable_to_create_target)

MaskRay wrote:
> Can `MOFI` be null? (i.e. can createMCObjectFileInfo guarantee no-null return 
> value?)
> 
> Consider moving the construction below the checks.
You're right. The `createMCObjectFileInfo` always returns a no-null value. I'll 
move the construction and remove the check for `MOFI`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101921

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

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

2021-05-21 Thread Philipp Krones via Phabricator via lldb-commits

flip1995 updated this revision to Diff 345432.
flip1995 added a comment.

- [MC] Don't check if constructed MOFI is a nullptr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101921

Files:
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/tools/driver/cc1as_main.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
  llvm/include/llvm/MC/MCContext.h
  llvm/include/llvm/Support/TargetRegistry.h
  llvm/lib/CodeGen/MachineModuleInfo.cpp
  llvm/lib/DWARFLinker/DWARFStreamer.cpp
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCDisassembler/Disassembler.cpp
  llvm/lib/Object/ModuleSymbolTable.cpp
  llvm/tools/llvm-cfi-verify/lib/FileAnalysis.cpp
  llvm/tools/llvm-cfi-verify/lib/FileAnalysis.h
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-exegesis/lib/Analysis.cpp
  llvm/tools/llvm-exegesis/lib/Analysis.h
  llvm/tools/llvm-exegesis/lib/LlvmState.cpp
  llvm/tools/llvm-exegesis/lib/SnippetFile.cpp
  llvm/tools/llvm-jitlink/llvm-jitlink.cpp
  llvm/tools/llvm-mc-assemble-fuzzer/llvm-mc-assemble-fuzzer.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-ml/Disassembler.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-objdump/MachODump.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-profgen/ProfiledBinary.cpp
  llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
  llvm/tools/sancov/sancov.cpp
  llvm/unittests/CodeGen/MachineInstrTest.cpp
  llvm/unittests/CodeGen/MachineOperandTest.cpp
  llvm/unittests/CodeGen/TestAsmPrinter.cpp
  llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
  llvm/unittests/MC/DwarfLineTables.cpp
  llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
  mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp

Index: mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
===
--- mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
+++ mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
@@ -168,9 +168,10 @@
   target->createMCAsmInfo(*mri, this->triple, mcOptions));
   mai->setRelaxELFRelocations(true);
 
-  llvm::MCObjectFileInfo mofi;
-  llvm::MCContext ctx(triple, mai.get(), mri.get(), &mofi, &srcMgr, &mcOptions);
-  mofi.initMCObjectFileInfo(ctx, /*PIC=*/false, /*LargeCodeModel=*/false);
+  llvm::MCContext ctx(triple, mai.get(), mri.get(), &srcMgr, &mcOptions);
+  std::unique_ptr mofi(target->createMCObjectFileInfo(
+  ctx, /*PIC=*/false, /*LargeCodeModel=*/false));
+  ctx.setObjectFileInfo(mofi.get());
 
   SmallString<128> cwd;
   if (!llvm::sys::fs::current_path(cwd))
Index: llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
===
--- llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
+++ llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
@@ -62,6 +62,7 @@
   std::unique_ptr MRI;
   std::unique_ptr MUPMAI;
   std::unique_ptr MII;
+  std::unique_ptr MOFI;
   std::unique_ptr Str;
   std::unique_ptr Parser;
   std::unique_ptr Ctx;
@@ -74,7 +75,6 @@
   const Target *TheTarget;
 
   const MCTargetOptions MCOptions;
-  MCObjectFileInfo MOFI;
 
   SystemZAsmLexerTest() {
 // We will use the SystemZ triple, because of missing
@@ -112,9 +112,11 @@
 SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
 EXPECT_EQ(Buffer, nullptr);
 
-Ctx.reset(new MCContext(Triple, MUPMAI.get(), MRI.get(), &MOFI, STI.get(),
-&SrcMgr, &MCOptions));
-MOFI.initMCObjectFileInfo(*Ctx, /*PIC=*/false, /*LargeCodeModel=*/false);
+Ctx.reset(new MCContext(Triple, MUPMAI.get(), MRI.get(), STI.get(), &SrcMgr,
+&MCOptions));
+MOFI.reset(TheTarget->createMCObjectFileInfo(*Ctx, /*PIC=*/false,
+ /*LargeCodeModel=*/false));
+Ctx->setObjectFileInfo(MOFI.get());
 
 Str.reset(TheTarget->createNullStreamer(*Ctx));
 
Index: llvm/unittests/MC/DwarfLineTables.cpp
===
--- llvm/unittests/MC/DwarfLineTables.cpp
+++ llvm/unittests/MC/DwarfLineTables.cpp
@@ -41,7 +41,7 @@
 MCTargetOptions MCOptions;
 MAI.reset(TheTarget->createMCAsmInfo(*MRI, TripleName, MCOptions));
 Ctx = std::make_unique(Triple(TripleName), MAI.get(), MRI.get(),
-  /*MOFI=*/nullptr, /*MSTI=*/nullptr);
+  /*MSTI=*/nullptr);
   }
 
   operator bool() { return Ctx.get(); }
Index: llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
===
--- llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
+++ llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
@@ -464,9 +464,10 @@
 return make_error("no target machine for target " + TripleName,

[Lldb-commits] [PATCH] D102630: [lit] Stop using PATH to lookup clang/lld/lldb unless requested

2021-05-21 Thread James Henderson via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa1e656585578: [lit] Stop using PATH to lookup clang/lld/lldb 
unless requested (authored by jhenderson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102630

Files:
  lldb/test/Shell/helper/toolchain.py
  llvm/utils/lit/lit/llvm/config.py

Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -401,10 +401,11 @@
 
 self.add_err_msg_substitutions()
 
-def use_llvm_tool(self, name, search_env=None, required=False, quiet=False):
+def use_llvm_tool(self, name, search_env=None, required=False, quiet=False,
+  use_installed=False):
 """Find the executable program 'name', optionally using the specified
-environment variable as an override before searching the
-configuration's PATH."""
+environment variable as an override before searching the build directory
+and then optionally the configuration's PATH."""
 # If the override is specified in the environment, use it without
 # validation.
 tool = None
@@ -412,7 +413,11 @@
 tool = self.config.environment.get(search_env)
 
 if not tool:
-# Otherwise look in the path.
+# Use the build directory version.
+tool = lit.util.which(name, self.config.llvm_tools_dir)
+
+if not tool and use_installed:
+# Otherwise look in the path, if enabled.
 tool = lit.util.which(name, self.config.environment['PATH'])
 
 if required and not tool:
@@ -429,11 +434,11 @@
 return tool
 
 def use_clang(self, additional_tool_dirs=[], additional_flags=[],
-  required=True):
+  required=True, use_installed=False):
 """Configure the test suite to be able to invoke clang.
 
 Sets up some environment variables important to clang, locates a
-just-built or installed clang, and add a set of standard
+just-built or optionally an installed clang, and add a set of standard
 substitutions useful to any test suite that makes use of clang.
 
 """
@@ -497,7 +502,8 @@
 
 # Discover the 'clang' and 'clangcc' to use.
 self.config.clang = self.use_llvm_tool(
-'clang', search_env='CLANG', required=required)
+'clang', search_env='CLANG', required=required,
+use_installed=use_installed)
 if self.config.clang:
   self.config.available_features.add('clang')
   builtin_include_dir = self.get_clang_builtin_include_dir(
@@ -569,11 +575,12 @@
 (' %clang-cl ',
  '''\"*** invalid substitution, use '%clang_cl'. ***\"'''))
 
-def use_lld(self, additional_tool_dirs=[], required=True):
+def use_lld(self, additional_tool_dirs=[], required=True,
+use_installed=False):
 """Configure the test suite to be able to invoke lld.
 
 Sets up some environment variables important to lld, locates a
-just-built or installed lld, and add a set of standard
+just-built or optionally an installed lld, and add a set of standard
 substitutions useful to any test suite that makes use of lld.
 
 """
@@ -593,12 +600,16 @@
 
 self.with_environment('LD_LIBRARY_PATH', paths, append_path=True)
 
-# Discover the 'clang' and 'clangcc' to use.
+# Discover the LLD executables to use.
 
-ld_lld = self.use_llvm_tool('ld.lld', required=required)
-lld_link = self.use_llvm_tool('lld-link', required=required)
-ld64_lld = self.use_llvm_tool('ld64.lld', required=required)
-wasm_ld = self.use_llvm_tool('wasm-ld', required=required)
+ld_lld = self.use_llvm_tool('ld.lld', required=required,
+use_installed=use_installed)
+lld_link = self.use_llvm_tool('lld-link', required=required,
+  use_installed=use_installed)
+ld64_lld = self.use_llvm_tool('ld64.lld', required=required,
+  use_installed=use_installed)
+wasm_ld = self.use_llvm_tool('wasm-ld', required=required,
+ use_installed=use_installed)
 
 was_found = ld_lld and lld_link and ld64_lld and wasm_ld
 tool_substitutions = []
Index: lldb/test/Shell/helper/toolchain.py
===
--- lldb/test/Shell/helper/toolchain.py
+++ lldb/test/Shell/helper/toolchain.py
@@ -147,14 +147,14 @@
 
 llvm_config.use_clang(additional_flags=['--target=specify-a-target-or-use-a-_host-substitution'],
   additional_tool_dirs=additional_tool_dirs,
-

[Lldb-commits] [PATCH] D102630: [lit] Stop using PATH to lookup clang/lld/lldb unless requested

2021-05-21 Thread James Henderson via Phabricator via lldb-commits

jhenderson created this revision.
jhenderson added reviewers: JDevlieghere, aprantl, MaskRay, thopre, dblaikie, 
teemperor.
Herald added subscribers: usaxena95, kadircet, delcypher.
jhenderson requested review of this revision.
Herald added subscribers: lldb-commits, ilya-biryukov, aheejin.
Herald added projects: LLDB, LLVM.

This patch stops lit from looking on the PATH for clang, lld and other users of 
use_llvm_tool (currently only the debuginfo-tests) unless the call explicitly 
requests to opt into using the PATH. When not opting in, tests will only look 
in the build directory.

See the mailing list thread starting from 
https://lists.llvm.org/pipermail/llvm-dev/2021-May/150421.html.

`use_clang` is used from:

- The main clang tests (we don't want to test the installed version here, only 
the built version).
- The debuginfo tests (this one is possibly debatable, but the conversation in 
D95339  suggests we should only use the build 
version in a monorepo setting; see also D96511 
).
- The clangd testing (although rGc29513f7e023f125c6d221db179dc40b79e5c074 
 suggests 
clang is not actually used).
- The lldb shell testing (this appears to deliberately allow using the 
installed version).

`use_lld` is used from:

- The main lld tests (we don't want to test the installed version here, only 
the built version).
- The lldb shell testing (this appears to deliberately allow using the 
installed version).

`use_llvm_tool` is used from:

- `use_lld` and `use_clang`
- The debuginfo_tests to lookup LLDB (see above reasoning for clang)
- The tests for lit itself (not in a way where the behaviour change is relevant)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102630

Files:
  lldb/test/Shell/helper/toolchain.py
  llvm/utils/lit/lit/llvm/config.py

Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -401,10 +401,11 @@
 
 self.add_err_msg_substitutions()
 
-def use_llvm_tool(self, name, search_env=None, required=False, quiet=False):
+def use_llvm_tool(self, name, search_env=None, required=False, quiet=False,
+  use_installed=False):
 """Find the executable program 'name', optionally using the specified
-environment variable as an override before searching the
-configuration's PATH."""
+environment variable as an override before searching the build directory
+and then optionally the configuration's PATH."""
 # If the override is specified in the environment, use it without
 # validation.
 tool = None
@@ -412,7 +413,11 @@
 tool = self.config.environment.get(search_env)
 
 if not tool:
-# Otherwise look in the path.
+# Use the build directory version.
+tool = lit.util.which(name, self.config.llvm_tools_dir)
+
+if not tool and use_installed:
+# Otherwise look in the path, if enabled.
 tool = lit.util.which(name, self.config.environment['PATH'])
 
 if required and not tool:
@@ -429,11 +434,11 @@
 return tool
 
 def use_clang(self, additional_tool_dirs=[], additional_flags=[],
-  required=True):
+  required=True, use_installed=False):
 """Configure the test suite to be able to invoke clang.
 
 Sets up some environment variables important to clang, locates a
-just-built or installed clang, and add a set of standard
+just-built or optionally an installed clang, and add a set of standard
 substitutions useful to any test suite that makes use of clang.
 
 """
@@ -497,7 +502,8 @@
 
 # Discover the 'clang' and 'clangcc' to use.
 self.config.clang = self.use_llvm_tool(
-'clang', search_env='CLANG', required=required)
+'clang', search_env='CLANG', required=required,
+use_installed=use_installed)
 if self.config.clang:
   self.config.available_features.add('clang')
   builtin_include_dir = self.get_clang_builtin_include_dir(
@@ -569,11 +575,12 @@
 (' %clang-cl ',
  '''\"*** invalid substitution, use '%clang_cl'. ***\"'''))
 
-def use_lld(self, additional_tool_dirs=[], required=True):
+def use_lld(self, additional_tool_dirs=[], required=True,
+use_installed=False):
 """Configure the test suite to be able to invoke lld.
 
 Sets up some environment variables important to lld, locates a
-just-built or installed lld, and add a set of standard
+just-built or optionally an installed lld, and add a set of standard
 substitutions useful to any test suite that makes use of lld.
 
 """
@@ -593,12 +60

[Lldb-commits] [PATCH] D102140: [ppc64le] [lldb] [testsuite] Fix false FAILs on ppc64* with no hw watchpoints

2021-05-21 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

Is there any command we can run to directly get the number of h/w watchpoints?

I'm thinking that a lot of these are:

  try:
doThing()
  except:
if we got this specific error

Then you could have this at the start of each test instead:

  if not self.HasWatchpoints():
 self.skipTest(...)

Where HasWatchpoints runs that command. If there isn't a specific command you 
could do something silly like watch `main` and see if it succeeds then remove 
the watchpoint.

Failing that you could make some `skipIfErrorNoWatchpoints(err)` just to reduce 
the duplication.




Comment at: 
lldb/test/API/commands/watchpoints/hello_watchpoint/TestMyFirstWatchpoint.py:68
+except:
+if self.getArchitecture() == 'powerpc64le' \
+   and "\nerror: Target supports (0) hardware watchpoint slots.\n" 
\

Is the arch check here needed, also should it include powerpc64be? (not very 
familiar with powerpc but your title says `ppc64*`)

I don't think it would harm to not check the arch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102140

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

[Lldb-commits] [PATCH] D102140: [ppc64le] [lldb] [testsuite] Fix false FAILs on ppc64le with no hw watchpoints

2021-05-21 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added inline comments.



Comment at: 
lldb/test/API/commands/watchpoints/hello_watchpoint/TestMyFirstWatchpoint.py:68
+except:
+if self.getArchitecture() == 'powerpc64le' \
+   and "\nerror: Target supports (0) hardware watchpoint slots.\n" 
\

jankratochvil wrote:
> DavidSpickett wrote:
> > Is the arch check here needed, also should it include powerpc64be? (not 
> > very familiar with powerpc but your title says `ppc64*`)
> > 
> > I don't think it would harm to not check the arch.
> > Is the arch check here needed, also should it include powerpc64be? (not 
> > very familiar with powerpc but your title says `ppc64*`)
> 
> ppc64be is not supported by LLDB (and I expect it will never be as PowerPC 
> has moved BE->LE).
> 
> 
> > I don't think it would harm to not check the arch.
> 
> I think it would harm. As if there is a regression LLDB falsely reports 0 
> watchpoints on all arches the testsuite will not find it out. Sure this 
> possible uncaught regression affects also PPC but one cannot do anything with 
> it there.
```
I think it would harm. As if there is a regression LLDB falsely reports 0 
watchpoints on all arches the testsuite will not find it out. Sure this 
possible uncaught regression affects also PPC but one cannot do anything with 
it there.
```

True, I didn't think of it that way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102140

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

[Lldb-commits] [PATCH] D102140: [ppc64le] [lldb] [testsuite] Fix false FAILs on ppc64* with no hw watchpoints

2021-05-21 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil planned changes to this revision.
jankratochvil marked an inline comment as done.
jankratochvil added a comment.

In D102140#2768660 , @DavidSpickett 
wrote:

> Is there any command we can run to directly get the number of h/w watchpoints?

Not aware of, I will check and/or code some, thanks for the review.

Comment at: 
lldb/test/API/commands/watchpoints/hello_watchpoint/TestMyFirstWatchpoint.py:68
+except:
+if self.getArchitecture() == 'powerpc64le' \
+   and "\nerror: Target supports (0) hardware watchpoint slots.\n" 
\

DavidSpickett wrote:
> Is the arch check here needed, also should it include powerpc64be? (not very 
> familiar with powerpc but your title says `ppc64*`)
> 
> I don't think it would harm to not check the arch.
> Is the arch check here needed, also should it include powerpc64be? (not very 
> familiar with powerpc but your title says `ppc64*`)

ppc64be is not supported by LLDB (and I expect it will never be as PowerPC has 
moved BE->LE).

> I don't think it would harm to not check the arch.

I think it would harm. As if there is a regression LLDB falsely reports 0 
watchpoints on all arches the testsuite will not find it out. Sure this 
possible uncaught regression affects also PPC but one cannot do anything with 
it there.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102140

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

[Lldb-commits] [PATCH] D102630: [lit] Stop using PATH to lookup clang/lld/lldb unless requested

2021-05-21 Thread Thomas Preud'homme via Phabricator via lldb-commits

thopre added a comment.

In D102630#2764705 , @dblaikie wrote:

> @thopre - as an aside: It'd be helpful if you could include some text in the 
> text box when marking something "approved" through phabricator. There's a 
> bug/limitation that approvals without text don't produce email to the mailing 
> list - so it looks like a patch hasn't been reviewed yet (or has been 
> committed without review).

Oh sorry, I was not aware of that limitation. Will be careful to include a 
message in the future


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102630

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

[Lldb-commits] [PATCH] D102140: [ppc64le] [lldb] [testsuite] Fix false FAILs on ppc64le with no hw watchpoints

2021-05-21 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

> If there isn't a specific command you could do something silly like watch 
> main and see if it succeeds then remove the watchpoint.

Or you could watch address 0, there's no requirement for it to be a symbol.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102140

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

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

2021-05-21 Thread Fangrui Song via Phabricator via lldb-commits

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

LGTM. This looks like an improvement because it avoids a temporary 
`MCObjectFileInfo MOFI;` (which appeared to be initialized in two subsequent 
calls) in numerous places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101921

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

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

2021-05-21 Thread Michael Benfield via Phabricator via lldb-commits

mbenfield updated this revision to Diff 346904.
mbenfield added a comment.
Herald added subscribers: llvm-commits, lldb-commits, kbarton, hiraditya, 
nemanjai.
Herald added projects: LLDB, LLVM.

Don't warn for reference or dependent types (fixing false positives).

Also fix a few places where this warning is correctly triggered. In a couple
places I did that by adding __attribute__((unused)) rather than removing the
unused variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p12.cpp
  clang/test/CodeGen/2007-10-30-Volatile.c
  clang/test/CodeGen/X86/x86_32-xsave.c
  clang/test/CodeGen/X86/x86_64-xsave.c
  clang/test/CodeGen/builtins-arm.c
  clang/test/CodeGen/builtins-riscv.c
  clang/test/FixIt/fixit.cpp
  clang/test/Misc/warning-wall.c
  clang/test/Sema/shift.c
  clang/test/Sema/vector-gcc-compat.c
  clang/test/Sema/vector-gcc-compat.cpp
  clang/test/Sema/warn-unused-but-set-parameters.c
  clang/test/Sema/warn-unused-but-set-variables.c
  clang/test/SemaCXX/goto.cpp
  clang/test/SemaCXX/shift.cpp
  clang/test/SemaCXX/sizeless-1.cpp
  clang/test/SemaCXX/warn-unused-but-set-parameters-cpp.cpp
  clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
  clang/test/SemaObjC/foreach.m
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
  llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
  llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
  llvm/lib/Target/X86/X86FloatingPoint.cpp
  llvm/utils/benchmark/src/complexity.cc

Index: llvm/utils/benchmark/src/complexity.cc
===
--- llvm/utils/benchmark/src/complexity.cc
+++ llvm/utils/benchmark/src/complexity.cc
@@ -76,7 +76,6 @@
 LeastSq MinimalLeastSq(const std::vector& n,
const std::vector& time,
BigOFunc* fitting_curve) {
-  double sigma_gn = 0.0;
   double sigma_gn_squared = 0.0;
   double sigma_time = 0.0;
   double sigma_time_gn = 0.0;
@@ -84,7 +83,6 @@
   // Calculate least square fitting parameter
   for (size_t i = 0; i < n.size(); ++i) {
 double gn_i = fitting_curve(n[i]);
-sigma_gn += gn_i;
 sigma_gn_squared += gn_i * gn_i;
 sigma_time += time[i];
 sigma_time_gn += time[i] * gn_i;
Index: llvm/lib/Target/X86/X86FloatingPoint.cpp
===
--- llvm/lib/Target/X86/X86FloatingPoint.cpp
+++ llvm/lib/Target/X86/X86FloatingPoint.cpp
@@ -1526,7 +1526,7 @@
 
 // Scan the assembly for ST registers used, defined and clobbered. We can
 // only tell clobbers from defs by looking at the asm descriptor.
-unsigned STUses = 0, STDefs = 0, STClobbers = 0, STDeadDefs = 0;
+unsigned STUses = 0, STDefs = 0, STClobbers = 0;
 unsigned NumOps = 0;
 SmallSet FRegIdx;
 unsigned RCID;
@@ -1559,8 +1559,6 @@
   case InlineAsm::Kind_RegDef:
   case InlineAsm::Kind_RegDefEarlyClobber:
 STDefs |= (1u << STReg);
-if (MO.isDead())
-  STDeadDefs |= (1u << STReg);
 break;
   case InlineAsm::Kind_Clobber:
 STClobbers |= (1u << STReg);
Index: llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
===
--- llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
+++ llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
@@ -415,5 +415,5 @@
   }
 
   BlockSizes.clear();
-  return true;
+  return EverMadeChange;
 }
Index: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
===
--- llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
+++ llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
@@ -1247,7 +1247,7 @@
   if (LoLoop.Preheader)
 LoLoop.Start = SearchForStart(LoLoop.Preheader);
   else
-return false;
+return Changed;
 
   // Find the low-overhead loop components and decide whether or not to fall
   // back to a normal loop. Also look for a vctp instructions and decide
@@ -1281,7 +1281,7 @@
   LLVM_DEBUG(LoLoop.dump());
   if (!LoLoop.FoundAllComponents()) {
 LLVM_DEBUG(dbgs() << "ARM Loops: Didn't find loop start, update, end\n");
-return false;
+return Changed;
   }
 
   assert(LoLoop.Start->getOpcode() != ARM::t2WhileLoopStart &&
Index: llvm/lib/Target/ARM/ARMISelLowering.cpp
===
--- llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -6

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

2021-05-21 Thread Dávid Bolvanský via Phabricator via lldb-commits

xbolva00 added a comment.

>> Also fix a few places where this warning is correctly triggered.

Create new patch please - dont mix fixes with new warning within one patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[Lldb-commits] [PATCH] D102904: [LoopNest][LoopFlatten] Change LoopFlattenPass to LoopNest pass

2021-05-21 Thread Yueh-Ting Chen via Phabricator via lldb-commits

eopXD updated this revision to Diff 346965.
eopXD added a comment.
Herald added subscribers: cfe-commits, libcxx-commits, openmp-commits, 
lldb-commits, Sanitizers, shabalin, jsmolens, eric-k256, dcaballe, cota, 
mravishankar, teijeong, frasercrmck, dexonsmith, rdzhabarov, tatianashp, 
wenlei, lxfind, dang, okura, bbn, jdoerfert, msifontes, sstefan1, jurahul, 
kuter, Kayjukh, grosul1, Joonsoo, stephenneuendorffer, kerbowa, liufengdb, 
aartbik, lucyrfox, mgester, arpith-jacob, csigg, nicolasvasilache, antiagainst, 
shauheen, rriddle, mehdi_amini, luismarques, apazos, sameer.abuasal, pengfei, 
s.egerton, dmgreen, Jim, mstorsjo, jocewei, rupprecht, PkmX, jfb, the_o, 
brucehoult, MartinMosbeck, rogfer01, steven_wu, atanasyan, edward-jones, 
zzheng, MaskRay, jrtc27, gbedwell, martong, delcypher, niosHD, cryptoad, 
sabuasal, simoncook, johnrusso, rbar, asb, fedor.sergeev, kbarton, aheejin, 
jgravelle-google, arichardson, sbc100, mgorny, nhaehnle, jvesely, nemanjai, 
sdardis, emaste, jyknight, dschuff, arsenm.
Herald added a reviewer: andreadb.
Herald added a reviewer: alexshap.
Herald added a reviewer: shafik.
Herald added a reviewer: rupprecht.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: jhenderson.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: sstefan1.
Herald added a reviewer: nicolasvasilache.
Herald added a reviewer: herhut.
Herald added a reviewer: rriddle.
Herald added a reviewer: aartbik.
Herald added a reviewer: sscalpone.
Herald added a reviewer: ftynse.
Herald added a reviewer: aaron.ballman.
Herald added a reviewer: baziotis.
Herald added a reviewer: aartbik.
Herald added a reviewer: aartbik.
Herald added a reviewer: int3.
Herald added a reviewer: sjarus.
Herald added a reviewer: gkm.
Herald added projects: clang, Sanitizers, LLDB, libc++, OpenMP, libc++abi, 
MLIR, lld-macho, clang-tools-extra.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.
Herald added a reviewer: lld-macho.

Apply clang-tidy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102904

Files:
  SECURITY.md
  clang-tools-extra/clang-tidy/modernize/RawStringLiteralCheck.cpp
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/arm_mve.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/ExternalSemaSource.h
  clang/include/clang/Sema/MultiplexExternalSemaSource.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Basic/Targets/M68k.cpp
  clang/lib/Basic/Targets/M68k.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenPGO.cpp
  clang/lib/CodeGen/CodeGenPGO.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/M68k.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Interpreter/IncrementalExecutor.cpp
  clang/lib/Interpreter/IncrementalExecutor.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/MultiplexExternalSemaSource.cpp
  clang/lib/Sema/SemaAccess.cpp
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/AST/ast-dump-constant-var.cu
  clang/test/ClangScanDeps/Inputs/modules_inferred_cdb.json
  clang/test/ClangScanDeps/modules-inferred-explicit-build.m
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector-constrained.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_rdffr.c
  clang/test/CodeGen/asan-globals-alias.cpp
  clang/test/CodeGen/asan-globals-odr.cpp
  clang/test/CodeGen/asan-globals.cpp
  clang/test/CodeGen/asan-static-odr.cpp
  clang/test/CodeGen/attr-weakref2.c
  clang/test/CodeGen/pre-ra-sched.c
  clang/test/CodeGen/semantic-interposition.c
  clang/test/CodeGenCUDA/device-stub.cu
  clang/test/CodeGenCUDA/device-var-linkage.cu
  clang/test/CodeGenCUDA/host-used-device-var.cu
  clang/test/CodeGenCUDA/managed-var.cu
  clang/test/CodeGenCUDA/static-device-var-rdc.cu
  clang/test/CodeGenCXX/debug-info-byval.cpp
  clang/test/CodeGenCXX/wasm-eh.cpp
  clang/test/CodeGenOpenCLCXX/addrspace-constructors.clcpp
  clang/test/Driver/arm-implicit-it.s
  clang/test/Driver/arm-target-as-mimplicit-it.s
  clang/test/Driver/as-options.s
  clang/test/Driver/clang-offload-bundler.c
  clang/test/Driver/lto.c
  clang/test/Driver/m68k-fixed-register.c

[Lldb-commits] [PATCH] D101128: [lldb-vscode] only report long running progress events

2021-05-21 Thread Greg Clayton via Phabricator via lldb-commits

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

The only last nit is we are passing the report progress callback around all 
over the place. I think it would be nicer to just make a static function that 
we can call and remove the passing of the instance variable for the report 
callback all over to the event classes.




Comment at: lldb/tools/lldb-vscode/ProgressEvent.cpp:27
+ const ProgressEvent *prev_event)
+: m_progress_id(progress_id), m_message(message.str()) {
+  if (completed == total) {

So we are copying the string for every event? I thought we only needed this for 
the start event?



Comment at: lldb/tools/lldb-vscode/ProgressEvent.cpp:28-57
+  if (completed == total) {
+m_percentage = 100;
+  } else if (total != UINT64_MAX) {
+m_percentage = std::min(
+(uint32_t)((long double)completed / (long double)total * 100.0),
+(uint32_t)99);
+  }

I would be nice to simplify this down a bit as we have both "completed" being 
used along with "prev_event" to try and figure out what the event is. We 
probably don't need "long double" for calculating the percentage either. It 
would be clearer to maybe do:

```
const bool calculate_percentage = total != UINT64_MAX;
if (completed == 0) {
  // Start event
  m_event_type = progressStart;
  // Wait a bit before reporting the start event in case in completes really 
quickly.
  m_minimum_allowed_report_time = m_creation_time + 
kStartProgressEventReportDelay;
  if (calculate_percentage)
m_precentage = 0;
} else if (completed == total) {
  // End event
  m_event_type = progressEnd;
  // We should report the end event right away.
  m_minimum_allowed_report_time = std::chrono::seconds::zero();
  if (calculate_percentage)
m_precentage = 100;
} else {
  // Update event
  assert(prev_event);
  m_percentage = std::min(
(uint32_t)((double)completed / (double)total * 100.0),
(uint32_t)99);
  if (prev_event->Reported()) {
// Add a small delay between reports
m_minimum_allowed_report_time =
prev_event->m_minimum_allowed_report_time +
kUpdateProgressEventReportDelay; 
  } else {
// We should use the previous timestamp, as it's still pending
m_minimum_allowed_report_time =
prev_event->m_minimum_allowed_report_time; 
  }
}
```



Comment at: lldb/tools/lldb-vscode/ProgressEvent.cpp:147
+  if (m_last_update_event)
+m_last_update_event->Report(m_report_callback);
+  return true;

Should we return the result of 
"m_last_update_event->Report(m_report_callback);" here?



Comment at: lldb/tools/lldb-vscode/ProgressEvent.cpp:157
+  uint64_t completed, uint64_t total) {
+  if (Optional event = ProgressEvent::Create(
+  progress_id, message, completed, total, &GetMostRecentEvent())) {

Should we add a safeguard and check m_finished first?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101128

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

[Lldb-commits] [PATCH] D102942: Remove or use variables which are unused but set.

2021-05-21 Thread Michael Benfield via Phabricator via lldb-commits

mbenfield created this revision.
Herald added subscribers: pengfei, lebedev.ri, kbarton, hiraditya, nemanjai.
Herald added a reviewer: lebedev.ri.
mbenfield requested review of this revision.
Herald added projects: LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits.

This is in preparation for the -Wunused-but-set-variable warning.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102942

Files:
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
  llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
  llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
  llvm/lib/Target/X86/X86FloatingPoint.cpp
  llvm/utils/benchmark/src/complexity.cc

Index: llvm/utils/benchmark/src/complexity.cc
===
--- llvm/utils/benchmark/src/complexity.cc
+++ llvm/utils/benchmark/src/complexity.cc
@@ -76,7 +76,6 @@
 LeastSq MinimalLeastSq(const std::vector& n,
const std::vector& time,
BigOFunc* fitting_curve) {
-  double sigma_gn = 0.0;
   double sigma_gn_squared = 0.0;
   double sigma_time = 0.0;
   double sigma_time_gn = 0.0;
@@ -84,7 +83,6 @@
   // Calculate least square fitting parameter
   for (size_t i = 0; i < n.size(); ++i) {
 double gn_i = fitting_curve(n[i]);
-sigma_gn += gn_i;
 sigma_gn_squared += gn_i * gn_i;
 sigma_time += time[i];
 sigma_time_gn += time[i] * gn_i;
Index: llvm/lib/Target/X86/X86FloatingPoint.cpp
===
--- llvm/lib/Target/X86/X86FloatingPoint.cpp
+++ llvm/lib/Target/X86/X86FloatingPoint.cpp
@@ -1526,7 +1526,7 @@
 
 // Scan the assembly for ST registers used, defined and clobbered. We can
 // only tell clobbers from defs by looking at the asm descriptor.
-unsigned STUses = 0, STDefs = 0, STClobbers = 0, STDeadDefs = 0;
+unsigned STUses = 0, STDefs = 0, STClobbers = 0;
 unsigned NumOps = 0;
 SmallSet FRegIdx;
 unsigned RCID;
@@ -1559,8 +1559,6 @@
   case InlineAsm::Kind_RegDef:
   case InlineAsm::Kind_RegDefEarlyClobber:
 STDefs |= (1u << STReg);
-if (MO.isDead())
-  STDeadDefs |= (1u << STReg);
 break;
   case InlineAsm::Kind_Clobber:
 STClobbers |= (1u << STReg);
Index: llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
===
--- llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
+++ llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
@@ -415,5 +415,5 @@
   }
 
   BlockSizes.clear();
-  return true;
+  return EverMadeChange;
 }
Index: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
===
--- llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
+++ llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
@@ -1247,7 +1247,7 @@
   if (LoLoop.Preheader)
 LoLoop.Start = SearchForStart(LoLoop.Preheader);
   else
-return false;
+return Changed;
 
   // Find the low-overhead loop components and decide whether or not to fall
   // back to a normal loop. Also look for a vctp instructions and decide
@@ -1281,7 +1281,7 @@
   LLVM_DEBUG(LoLoop.dump());
   if (!LoLoop.FoundAllComponents()) {
 LLVM_DEBUG(dbgs() << "ARM Loops: Didn't find loop start, update, end\n");
-return false;
+return Changed;
   }
 
   assert(LoLoop.Start->getOpcode() != ARM::t2WhileLoopStart &&
Index: llvm/lib/Target/ARM/ARMISelLowering.cpp
===
--- llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -6746,12 +6746,10 @@
   return SDValue();
 // NEON has a 64-bit VMOV splat where each byte is either 0 or 0xff.
 uint64_t BitMask = 0xff;
-uint64_t Val = 0;
 unsigned ImmMask = 1;
 Imm = 0;
 for (int ByteNum = 0; ByteNum < 8; ++ByteNum) {
   if (((SplatBits | SplatUndef) & BitMask) == BitMask) {
-Val |= BitMask;
 Imm |= ImmMask;
   } else if ((SplatBits & BitMask) != 0) {
 return SDValue();
Index: llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp
@@ -180,7 +180,7 @@
   bool Changed = false;
   for (auto &BB : MF)
 Changed |= optimizeNZCVDefs(BB);
-  return true;
+  return Changed;
 }
 
 char AArch64PostSelectOptimize::ID = 0;
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
+++ lldb/

[Lldb-commits] [PATCH] D102942: Remove or use variables which are unused but set.

2021-05-21 Thread Michael Benfield via Phabricator via lldb-commits

mbenfield added a comment.
Herald added a subscriber: JDevlieghere.

Several variables I have just removed outright, including

llvm/lib/Target/X86/X86FloatingPoint.cpp: STDeadDefs
lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp: fp_bytes
lldb/source/Interpreter/CommandInterpreter.cpp: actual_cmd_name_len

Someone familiar with this code should probably look at these and determine 
whether these variables not being used could have lead to buggy behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102942

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

Re: [Lldb-commits] [lldb] 8dd1060 - [debugserver] Add platform cache support to improve performance.

2021-05-21 Thread Shafik Yaghmour via lldb-commits

So I guess we don’t use in class member initialization for m_platform b/c 
GetPlatform() can never be called before either Clear() or Detach()?

> On May 20, 2021, at 7:10 PM, Jonas Devlieghere via lldb-commits 
>  wrote:
> 
> 
> Author: kuperxu
> Date: 2021-05-20T19:10:46-07:00
> New Revision: 8dd106028b1533f0de03a1ffb4ea0dce40b5a2ff
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/8dd106028b1533f0de03a1ffb4ea0dce40b5a2ff
> DIFF: 
> https://github.com/llvm/llvm-project/commit/8dd106028b1533f0de03a1ffb4ea0dce40b5a2ff.diff
> 
> LOG: [debugserver] Add platform cache support to improve performance.
> 
> The dyld SPI used by debugserver (_dyld_process_info_create) has become
> much slower in macOS BigSur 11.3 causing a significant performance
> regression when attaching. This commit mitigates that by caching the
> result when calling the SPI to compute the platform.
> 
> Differential revision: https://reviews.llvm.org/D102833
> 
> Added: 
> 
> 
> Modified: 
>lldb/tools/debugserver/source/MacOSX/MachProcess.h
>lldb/tools/debugserver/source/MacOSX/MachProcess.mm
> 
> Removed: 
> 
> 
> 
> 
> diff  --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.h 
> b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
> index b295dfecec41..33c3d628a7a0 100644
> --- a/lldb/tools/debugserver/source/MacOSX/MachProcess.h
> +++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
> @@ -252,6 +252,7 @@ class MachProcess {
>  struct mach_o_information &inf);
>   JSONGenerator::ObjectSP FormatDynamicLibrariesIntoJSON(
>   const std::vector &image_infos);
> +  uint32_t GetPlatform();
>   /// Get the runtime platform from DYLD via SPI.
>   uint32_t GetProcessPlatformViaDYLDSPI();
>   /// Use the dyld SPI present in macOS 10.12, iOS 10, tvOS 10,
> @@ -378,6 +379,7 @@ class MachProcess {
> 
>   pid_t m_pid;   // Process ID of child process
>   cpu_type_t m_cpu_type; // The CPU type of this process
> +  uint32_t m_platform;   // The platform of this process
>   int m_child_stdin;
>   int m_child_stdout;
>   int m_child_stderr;
> 
> diff  --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm 
> b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
> index 0a6ef6161711..7eab2c6d185f 100644
> --- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
> +++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
> @@ -701,7 +701,7 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary 
> *options,
>   // DYLD_FORCE_PLATFORM=6. In that case, force the platform to
>   // macCatalyst and use the macCatalyst version of the host OS
>   // instead of the macOS deployment target.
> -  if (is_executable && GetProcessPlatformViaDYLDSPI() == 
> PLATFORM_MACCATALYST) {
> +  if (is_executable && GetPlatform() == PLATFORM_MACCATALYST) {
> info.platform = PLATFORM_MACCATALYST;
> std::string catalyst_version = GetMacCatalystVersionString();
> const char *major = catalyst_version.c_str();
> @@ -1094,6 +1094,12 @@ static bool 
> FBSAddEventDataToOptions(NSMutableDictionary *options,
>   bool privateCache;
> };
> 
> +uint32_t MachProcess::GetPlatform() {
> +  if (m_platform == 0)
> +m_platform = MachProcess::GetProcessPlatformViaDYLDSPI();
> +  return m_platform;
> +}
> +
> uint32_t MachProcess::GetProcessPlatformViaDYLDSPI() {
>   kern_return_t kern_ret;
>   uint32_t platform = 0;
> @@ -1140,7 +1146,7 @@ static bool 
> FBSAddEventDataToOptions(NSMutableDictionary *options,
>   int pointer_size = GetInferiorAddrSize(pid);
>   std::vector image_infos;
>   GetAllLoadedBinariesViaDYLDSPI(image_infos);
> -  uint32_t platform = GetProcessPlatformViaDYLDSPI();
> +  uint32_t platform = GetPlatform();
>   const size_t image_count = image_infos.size();
>   for (size_t i = 0; i < image_count; i++) {
> GetMachOInformationFromMemory(platform, image_infos[i].load_address,
> @@ -1160,7 +1166,7 @@ static bool 
> FBSAddEventDataToOptions(NSMutableDictionary *options,
> 
>   std::vector all_image_infos;
>   GetAllLoadedBinariesViaDYLDSPI(all_image_infos);
> -  uint32_t platform = GetProcessPlatformViaDYLDSPI();
> +  uint32_t platform = GetPlatform();
> 
>   std::vector image_infos;
>   const size_t macho_addresses_count = macho_addresses.size();
> @@ -1324,6 +1330,7 @@ static bool 
> FBSAddEventDataToOptions(NSMutableDictionary *options,
>   // Clear any cached thread list while the pid and task are still valid
> 
>   m_task.Clear();
> +  m_platform = 0;
>   // Now clear out all member variables
>   m_pid = INVALID_NUB_PROCESS;
>   if (!detaching)
> @@ -1615,6 +1622,7 @@ static bool 
> FBSAddEventDataToOptions(NSMutableDictionary *options,
> 
>   // NULL our task out as we have already restored all exception ports
>   m_task.Clear();
> +  m_platform = 0;
> 
>   // Clear out any notion of the process we once were
>   const bool detaching = true;
> 
> 
> 
> ___

[Lldb-commits] [PATCH] D102942: Remove or use variables which are unused but set.

2021-05-21 Thread David Blaikie via Phabricator via lldb-commits

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

Sure, all sounds good - if you can, please reach out to the authors of any of 
the semantics changing changes (the ones related to the `Changed` values in 
transformations) to see if they could add missing test coverage.




Comment at: llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp:183
 Changed |= optimizeNZCVDefs(BB);
-  return true;
+  return Changed;
 }

Might want to CC/respond to whoever wrote this to see if someone missed test 
coverage for this code.



Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:1247-1250
   if (LoLoop.Preheader)
 LoLoop.Start = SearchForStart(LoLoop.Preheader);
   else
+return Changed;

Maybe in a separate patch would be good to switch this around to:
```
if (!LoLoop.Preheader)
  return Changed;
LoLoop.Start = SearchForStart(LoLoop.Preheader);
```



Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:1250
   else
-return false;
+return Changed;
 

Also might be worth reaching out to authors to check that this change is 
intended & possibly tested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102942

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

[Lldb-commits] [PATCH] D100243: [LLDB][GUI] Expand selected thread tree item by default

2021-05-21 Thread walter erquinigo via Phabricator via lldb-commits

wallace added inline comments.
Herald added a reviewer: teemperor.



Comment at: 
lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py:38
+# Exit GUI.
+self.child.send(escape_key)
+self.expect_prompt()

create a method self.exit_gui to dedup some code, as you'll end up doing a lot 
of them as you write your tests

  def exit_gui(self):
self.child.send(chr(27).encode())
self.expect_prompt()

you can also create a method start_gui

  def start_gui(self):
self.child.sendline("gui")
self.child.send(chr(27).encode())

you could also later create a base class lldbgui_testcase similar to 
lldbvscode_testcase, where you can put all needed common code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100243

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

[Lldb-commits] [lldb] 5e32778 - [lldb] Match test dependencies name to other LLVM projects.

2021-05-21 Thread Shoaib Meenai via lldb-commits


Author: Daniel Rodríguez Troitiño
Date: 2021-05-21T00:10:27-07:00
New Revision: 5e327785da36fa3a00767cfea0a47fcb0f0814aa

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

LOG: [lldb] Match test dependencies name to other LLVM projects.

Other LLVM projects use the suffix `-depends` for the test dependencies,
however LLDB uses `-deps` and seems to be the only project under the
LLVM to do so.

In order to make the projects more homogeneous, switch all the
references to `lldb-test-deps` to `lldb-test-depends`.

Additionally, provide a compatibility target with the old name and
depending on the new name, in order to not break anyone workflow.

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/test/API/CMakeLists.txt
lldb/test/CMakeLists.txt
lldb/test/Shell/CMakeLists.txt
lldb/test/Unit/CMakeLists.txt
lldb/utils/lldb-dotest/CMakeLists.txt
lldb/utils/lldb-repro/CMakeLists.txt

Removed: 




diff  --git a/lldb/test/API/CMakeLists.txt b/lldb/test/API/CMakeLists.txt
index ce7531fd98665..7625a4e5e29c7 100644
--- a/lldb/test/API/CMakeLists.txt
+++ b/lldb/test/API/CMakeLists.txt
@@ -1,5 +1,5 @@
 add_custom_target(lldb-api-test-deps)
-add_dependencies(lldb-api-test-deps lldb-test-deps)
+add_dependencies(lldb-api-test-deps lldb-test-depends)
 
 add_lit_testsuites(LLDB-API
   ${CMAKE_CURRENT_SOURCE_DIR}
@@ -17,7 +17,7 @@ function(add_python_test_target name test_script args comment)
 COMMENT "${comment}"
 USES_TERMINAL
 )
-  add_dependencies(${name} lldb-test-deps)
+  add_dependencies(${name} lldb-test-depends)
 endfunction()
 
 # The default architecture with which to compile test executables is the

diff  --git a/lldb/test/CMakeLists.txt b/lldb/test/CMakeLists.txt
index c6b01c66a0ef2..584cacbb99c1a 100644
--- a/lldb/test/CMakeLists.txt
+++ b/lldb/test/CMakeLists.txt
@@ -31,12 +31,16 @@ string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} 
LLDB_LIBS_DIR ${LLVM_LIBRA
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TOOLS_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
 
 # Create a custom target to track test dependencies.
+add_custom_target(lldb-test-depends)
+set_target_properties(lldb-test-depends PROPERTIES FOLDER "lldb misc")
+
+# Create an alias for the legacy name of lldb-test-depends
 add_custom_target(lldb-test-deps)
-set_target_properties(lldb-test-deps PROPERTIES FOLDER "lldb misc")
+add_dependencies(lldb-test-deps lldb-test-depends)
 
 function(add_lldb_test_dependency)
   foreach(dependency ${ARGN})
-add_dependencies(lldb-test-deps ${dependency})
+add_dependencies(lldb-test-depends ${dependency})
   endforeach()
 endfunction(add_lldb_test_dependency)
 
@@ -201,7 +205,7 @@ add_lit_testsuite(check-lldb-reproducers-capture
   ${CMAKE_CURRENT_BINARY_DIR}/Shell
   PARAMS "lldb-run-with-repro=capture"
   EXCLUDE_FROM_CHECK_ALL
-  DEPENDS lldb-test-deps)
+  DEPENDS lldb-test-depends)
 
 # Add a lit test suite that runs the API & shell test by replaying a
 # reproducer.
@@ -211,7 +215,7 @@ add_lit_testsuite(check-lldb-reproducers
   ${CMAKE_CURRENT_BINARY_DIR}/Shell
   PARAMS "lldb-run-with-repro=replay"
   EXCLUDE_FROM_CHECK_ALL
-  DEPENDS lldb-test-deps)
+  DEPENDS lldb-test-depends)
 add_dependencies(check-lldb-reproducers check-lldb-reproducers-capture)
 
 if(LLDB_BUILT_STANDALONE)

diff  --git a/lldb/test/Shell/CMakeLists.txt b/lldb/test/Shell/CMakeLists.txt
index f0d7b9a34651b..123bee6164537 100644
--- a/lldb/test/Shell/CMakeLists.txt
+++ b/lldb/test/Shell/CMakeLists.txt
@@ -1,5 +1,5 @@
 add_custom_target(lldb-shell-test-deps)
-add_dependencies(lldb-shell-test-deps lldb-test-deps)
+add_dependencies(lldb-shell-test-deps lldb-test-depends)
 
 add_lit_testsuites(LLDB-SHELL
   ${CMAKE_CURRENT_SOURCE_DIR}

diff  --git a/lldb/test/Unit/CMakeLists.txt b/lldb/test/Unit/CMakeLists.txt
index 3233c0873c1fb..a592e1cb1a1fc 100644
--- a/lldb/test/Unit/CMakeLists.txt
+++ b/lldb/test/Unit/CMakeLists.txt
@@ -1,5 +1,5 @@
 add_custom_target(lldb-unit-test-deps)
-add_dependencies(lldb-unit-test-deps lldb-test-deps)
+add_dependencies(lldb-unit-test-deps lldb-test-depends)
 
 add_lit_testsuites(LLDB-UNIT
   ${CMAKE_CURRENT_SOURCE_DIR}

diff  --git a/lldb/utils/lldb-dotest/CMakeLists.txt 
b/lldb/utils/lldb-dotest/CMakeLists.txt
index 5b7522835acd1..aab3f363a54bd 100644
--- a/lldb/utils/lldb-dotest/CMakeLists.txt
+++ b/lldb/utils/lldb-dotest/CMakeLists.txt
@@ -1,6 +1,6 @@
 # Make lldb-dotest a custom target.
 add_custom_target(lldb-dotest)
-add_dependencies(lldb-dotest lldb-test-deps)
+add_dependencies(lldb-dotest lldb-test-depends)
 set_target_properties(lldb-dotest PROPERTIES FOLDER "lldb utils")
 
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)

diff  --git a/lldb/utils/lldb-rep

[Lldb-commits] [PATCH] D102889: [lldb] Match test dependencies name to other LLVM projects.

2021-05-21 Thread Shoaib Meenai via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5e327785da36: [lldb] Match test dependencies name to other 
LLVM projects. (authored by drodriguez, committed by smeenai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102889

Files:
  lldb/test/API/CMakeLists.txt
  lldb/test/CMakeLists.txt
  lldb/test/Shell/CMakeLists.txt
  lldb/test/Unit/CMakeLists.txt
  lldb/utils/lldb-dotest/CMakeLists.txt
  lldb/utils/lldb-repro/CMakeLists.txt

Index: lldb/utils/lldb-repro/CMakeLists.txt
===
--- lldb/utils/lldb-repro/CMakeLists.txt
+++ lldb/utils/lldb-repro/CMakeLists.txt
@@ -1,5 +1,5 @@
 add_custom_target(lldb-repro)
-add_dependencies(lldb-repro lldb-test-deps)
+add_dependencies(lldb-repro lldb-test-depends)
 set_target_properties(lldb-repro PROPERTIES FOLDER "lldb utils")
 
 # Generate lldb-repro Python script for each build mode.
Index: lldb/utils/lldb-dotest/CMakeLists.txt
===
--- lldb/utils/lldb-dotest/CMakeLists.txt
+++ lldb/utils/lldb-dotest/CMakeLists.txt
@@ -1,6 +1,6 @@
 # Make lldb-dotest a custom target.
 add_custom_target(lldb-dotest)
-add_dependencies(lldb-dotest lldb-test-deps)
+add_dependencies(lldb-dotest lldb-test-depends)
 set_target_properties(lldb-dotest PROPERTIES FOLDER "lldb utils")
 
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
Index: lldb/test/Unit/CMakeLists.txt
===
--- lldb/test/Unit/CMakeLists.txt
+++ lldb/test/Unit/CMakeLists.txt
@@ -1,5 +1,5 @@
 add_custom_target(lldb-unit-test-deps)
-add_dependencies(lldb-unit-test-deps lldb-test-deps)
+add_dependencies(lldb-unit-test-deps lldb-test-depends)
 
 add_lit_testsuites(LLDB-UNIT
   ${CMAKE_CURRENT_SOURCE_DIR}
Index: lldb/test/Shell/CMakeLists.txt
===
--- lldb/test/Shell/CMakeLists.txt
+++ lldb/test/Shell/CMakeLists.txt
@@ -1,5 +1,5 @@
 add_custom_target(lldb-shell-test-deps)
-add_dependencies(lldb-shell-test-deps lldb-test-deps)
+add_dependencies(lldb-shell-test-deps lldb-test-depends)
 
 add_lit_testsuites(LLDB-SHELL
   ${CMAKE_CURRENT_SOURCE_DIR}
Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -31,12 +31,16 @@
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TOOLS_DIR ${LLVM_RUNTIME_OUTPUT_INTDIR})
 
 # Create a custom target to track test dependencies.
+add_custom_target(lldb-test-depends)
+set_target_properties(lldb-test-depends PROPERTIES FOLDER "lldb misc")
+
+# Create an alias for the legacy name of lldb-test-depends
 add_custom_target(lldb-test-deps)
-set_target_properties(lldb-test-deps PROPERTIES FOLDER "lldb misc")
+add_dependencies(lldb-test-deps lldb-test-depends)
 
 function(add_lldb_test_dependency)
   foreach(dependency ${ARGN})
-add_dependencies(lldb-test-deps ${dependency})
+add_dependencies(lldb-test-depends ${dependency})
   endforeach()
 endfunction(add_lldb_test_dependency)
 
@@ -201,7 +205,7 @@
   ${CMAKE_CURRENT_BINARY_DIR}/Shell
   PARAMS "lldb-run-with-repro=capture"
   EXCLUDE_FROM_CHECK_ALL
-  DEPENDS lldb-test-deps)
+  DEPENDS lldb-test-depends)
 
 # Add a lit test suite that runs the API & shell test by replaying a
 # reproducer.
@@ -211,7 +215,7 @@
   ${CMAKE_CURRENT_BINARY_DIR}/Shell
   PARAMS "lldb-run-with-repro=replay"
   EXCLUDE_FROM_CHECK_ALL
-  DEPENDS lldb-test-deps)
+  DEPENDS lldb-test-depends)
 add_dependencies(check-lldb-reproducers check-lldb-reproducers-capture)
 
 if(LLDB_BUILT_STANDALONE)
Index: lldb/test/API/CMakeLists.txt
===
--- lldb/test/API/CMakeLists.txt
+++ lldb/test/API/CMakeLists.txt
@@ -1,5 +1,5 @@
 add_custom_target(lldb-api-test-deps)
-add_dependencies(lldb-api-test-deps lldb-test-deps)
+add_dependencies(lldb-api-test-deps lldb-test-depends)
 
 add_lit_testsuites(LLDB-API
   ${CMAKE_CURRENT_SOURCE_DIR}
@@ -17,7 +17,7 @@
 COMMENT "${comment}"
 USES_TERMINAL
 )
-  add_dependencies(${name} lldb-test-deps)
+  add_dependencies(${name} lldb-test-depends)
 endfunction()
 
 # The default architecture with which to compile test executables is the
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D102872: Fix lldb-server build failure on mips

2021-05-21 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

I assume this is caused by the removal of the specific 
`NativeRegisterContextLinux_mips` and you are building lldb on MIPS but you're 
going to connect to another debug server (gdb, custom stub etc.).

The problem with doing it this way is then if you build on a supported platform 
e.g. AArch64 `NativeRegisterContextLinux_arm64` we then have two copies of 
CreateHostNativeRegisterContextLinux. Somehow this isn't an error on my system, 
and it picks up the one you've added. Meaning that you can't debug a simple 
hello world.

See the comment in 
`lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h`. (though again 
I'd have expect a hard linker error but anyway) Also the reason CI didn't fail 
is that lldb is disabled for both of the builds.

I can see the utility in allowing people to build at least lldb (client) on any 
platform. Perhaps we surround this in the `#ifdef` that the old mips register 
context used? Or modify our cmake to allow lldb-server to be disabled. (easier 
said than done)


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

https://reviews.llvm.org/D102872

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

[Lldb-commits] [PATCH] D102872: Fix lldb-server build failure on mips

2021-05-21 Thread Khem Raj via Phabricator via lldb-commits

raj.khem added a comment.

In D102872#2773106 , @DavidSpickett 
wrote:

> I assume this is caused by the removal of the specific 
> `NativeRegisterContextLinux_mips` and you are building lldb on MIPS but 
> you're going to connect to another debug server (gdb, custom stub etc.).
>
> The problem with doing it this way is then if you build on a supported 
> platform e.g. AArch64 `NativeRegisterContextLinux_arm64` we then have two 
> copies of CreateHostNativeRegisterContextLinux. Somehow this isn't an error 
> on my system, and it picks up the one you've added. Meaning that you can't 
> debug a simple hello world.
>
> See the comment in 
> `lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h`. (though 
> again I'd have expect a hard linker error but anyway) Also the reason CI 
> didn't fail is that lldb is disabled for both of the builds.
>
> I can see the utility in allowing people to build at least lldb (client) on 
> any platform. Perhaps we surround this in the `#ifdef` that the old mips 
> register context used? Or modify our cmake to allow lldb-server to be 
> disabled. (easier said than done)

yes that will be ok. Although, I think for kind of devices mips is used in, 
lldb-server is more useful even if it will not support all features.


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

https://reviews.llvm.org/D102872

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

[Lldb-commits] [PATCH] D102208: Remove Windows editline from LLDB

2021-05-21 Thread Neal via Phabricator via lldb-commits

nealsid created this revision.
nealsid added reviewers: LLDB, kparzysz.
Herald added subscribers: hiraditya, krytarowski, mgorny.
nealsid requested review of this revision.
Herald added projects: LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits.

I don't mean to undo others' work but it looks like the hand-rolled EditLine 
for LLDB on Windows isn't used.  It'd be easier to make changes to bring the 
other platforms' Editline wrapper up to date (e.g. simplifying char vs wchar_t) 
without modifying/testing this one too.  I also modified an #ifdef check for 
the MSVC version number to increase it, as it happens on VS2019, too.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102208

Files:
  lldb/include/lldb/Host/Editline.h
  lldb/include/lldb/Host/windows/editlinewin.h
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/windows/EditLineWin.cpp
  llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp

Index: llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp
===
--- llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp
+++ llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp
@@ -299,7 +299,7 @@
   return getIfUnordered(dyn_cast(In));
 }
 
-#if !defined(_MSC_VER) || _MSC_VER >= 1924
+#if !defined(_MSC_VER) || _MSC_VER >= 1926
 // VS2017 has trouble compiling this:
 // error C2976: 'std::map': too few template arguments
 template 
Index: lldb/source/Host/windows/EditLineWin.cpp
===
--- lldb/source/Host/windows/EditLineWin.cpp
+++ /dev/null
@@ -1,349 +0,0 @@
-//===-- EditLineWin.cpp ---===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-// this file is only relevant for Visual C++
-#if defined(_WIN32)
-
-#include "lldb/Host/windows/windows.h"
-
-#include "lldb/Host/windows/editlinewin.h"
-#include "llvm/Support/ErrorHandling.h"
-#include 
-#include 
-
-// edit line EL_ADDFN function pointer type
-typedef unsigned char (*el_addfn_func)(EditLine *e, int ch);
-typedef const char *(*el_prompt_func)(EditLine *);
-
-// edit line wrapper binding container
-struct el_binding {
-  //
-  const char *name;
-  const char *help;
-  // function pointer to callback routine
-  el_addfn_func func;
-  // ascii key this function is bound to
-  const char *key;
-};
-
-// stored key bindings
-static std::vector _bindings;
-
-// TODO: this should in fact be related to the exact edit line context we create
-static void *clientData = NULL;
-
-// store the current prompt string
-// default to what we expect to receive anyway
-static const char *_prompt = "(lldb) ";
-
-#if !defined(_WIP_INPUT_METHOD)
-
-static char *el_get_s(char *buffer, int chars) { return gets_s(buffer, chars); }
-#else
-
-static void con_output(char _in) {
-  HANDLE hout = GetStdHandle(STD_OUTPUT_HANDLE);
-  DWORD written = 0;
-  // get the cursor position
-  CONSOLE_SCREEN_BUFFER_INFO info;
-  GetConsoleScreenBufferInfo(hout, &info);
-  // output this char
-  WriteConsoleOutputCharacterA(hout, &_in, 1, info.dwCursorPosition, &written);
-  // advance cursor position
-  info.dwCursorPosition.X++;
-  SetConsoleCursorPosition(hout, info.dwCursorPosition);
-}
-
-static void con_backspace(void) {
-  HANDLE hout = GetStdHandle(STD_OUTPUT_HANDLE);
-  DWORD written = 0;
-  // get cursor position
-  CONSOLE_SCREEN_BUFFER_INFO info;
-  GetConsoleScreenBufferInfo(hout, &info);
-  // nudge cursor backwards
-  info.dwCursorPosition.X--;
-  SetConsoleCursorPosition(hout, info.dwCursorPosition);
-  // blank out the last character
-  WriteConsoleOutputCharacterA(hout, " ", 1, info.dwCursorPosition, &written);
-}
-
-static void con_return(void) {
-  HANDLE hout = GetStdHandle(STD_OUTPUT_HANDLE);
-  DWORD written = 0;
-  // get cursor position
-  CONSOLE_SCREEN_BUFFER_INFO info;
-  GetConsoleScreenBufferInfo(hout, &info);
-  // move onto the new line
-  info.dwCursorPosition.X = 0;
-  info.dwCursorPosition.Y++;
-  SetConsoleCursorPosition(hout, info.dwCursorPosition);
-}
-
-static bool runBind(char _key) {
-  for (int i = 0; i < _bindings.size(); i++) {
-el_binding *bind = _bindings[i];
-if (bind->key[0] == _key) {
-  bind->func((EditLine *)-1, _key);
-  return true;
-}
-  }
-  return false;
-}
-
-// replacement get_s which is EL_BIND aware
-static char *el_get_s(char *buffer, int chars) {
-  //
-  char *head = buffer;
-  //
-  for (;; Sleep(10)) {
-//
-INPUT_RECORD _record;
-//
-DWORD _read = 0;
-if (ReadConsoleInputA(GetStdHandle(STD_INPUT_HANDLE), &_record, 1,
-  &_read) == FALSE)
-  break;
-// if we didn't read a key
-if (_read == 0)
-  continue;
-// only inter

[Lldb-commits] [PATCH] D102208: Remove Windows editline from LLDB

2021-05-21 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added inline comments.



Comment at: llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp:302
 
-#if !defined(_MSC_VER) || _MSC_VER >= 1924
+#if !defined(_MSC_VER) || _MSC_VER >= 1926
 // VS2017 has trouble compiling this:

This should be done in a separate change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102208

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

[Lldb-commits] [PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-05-21 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

@ychen Since this change the warning emitted in `AsmPrinterInlineAsm.cpp` about 
reserved registers no longer has location information for the file that the 
inline asm is in. Here's an example:

  void bar(void)
  {
  __asm__ __volatile__ ( "nop" : : : "sp");
  }
  
  ./bin/clang --target=arm-arm-none-eabi -march=armv7-m -c /tmp/test.c -o 
/dev/null

Before:

  /tmp/test.c:3:28: warning: inline asm clobber list contains reserved 
registers: SP [-Winline-asm]
  __asm__ __volatile__ ( "nop" : : : "sp");
 ^
  :1:1: note: instantiated into assembly here
  nop
  ^
  /tmp/test.c:3:28: note: Reserved registers on the clobber list may not be 
preserved across the asm statement, and clobbering them may lead to undefined 
behaviour.
  __asm__ __volatile__ ( "nop" : : : "sp");
 ^
  :1:1: note: instantiated into assembly here
  nop
  ^
  1 warning generated.

After:

  :1:1: warning: inline asm clobber list contains reserved 
registers: SP
  nop
  
  :1:1: note: Reserved registers on the clobber list may not be 
preserved across the asm statement, and clobbering them may lead to undefined 
behaviour.
  nop
  

(the reason it didn't break any tests is that we only check this message from 
IR and only look for the "contains reserved registers" line)

Can you guide me on how I might correct this?

My current impression is that since this code does `SrcMgr.PrintMessage(Loc, 
SourceMgr::DK_Warning, Msg)` it's one level too deep to get source information 
added to it automatically. And that if this code could use `DK_InlineAsm` that 
would help. However getting hold of the Diagnostic manager from MCContext isn't 
possible currently, perhaps that is by design?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

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

[Lldb-commits] [PATCH] D102208: Remove Windows editline from LLDB

2021-05-21 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added reviewers: stella.stamenova, amccarth.
teemperor added a comment.

No idea about how/if this is used, but I think Stella or Adrian are the ones 
that might be able to answer this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102208

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

[Lldb-commits] [PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

2021-05-21 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

  Can you guide me on how I might correct this?

Actually, now I see that MCContext has reportWarning/reportError and just needs 
a reportNote. I'll have something for you to review shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449

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

[Lldb-commits] [PATCH] D102208: Remove Windows editline from LLDB

2021-05-21 Thread Adrian McCarthy via Phabricator via lldb-commits

amccarth added a subscriber: zturner.
amccarth added a comment.

It looks like this EditLine port was added before I joined this project.  
@zturner may have worked on it, but I don't know/remember.

If it's actually unused, I have no objection to removing it.  I harbor some 
hope that Windows's steadily improving unix-like terminal implementation (which 
console programs can enable) will eventually make a Windows-port of EditLine 
trivial.

I concur that the MSVC version tweak should be a separate change.  If you 
remove that from this patch, I'll LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102208

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

[Lldb-commits] [PATCH] D102208: Remove Windows editline from LLDB

2021-05-21 Thread Neal via Phabricator via lldb-commits

nealsid updated this revision to Diff 344489.
nealsid added a comment.

Updated the diff after moving an unrelated change into another revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102208

Files:
  lldb/include/lldb/Host/Editline.h
  lldb/include/lldb/Host/windows/editlinewin.h
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/windows/EditLineWin.cpp

Index: lldb/source/Host/windows/EditLineWin.cpp
===
--- lldb/source/Host/windows/EditLineWin.cpp
+++ /dev/null
@@ -1,349 +0,0 @@
-//===-- EditLineWin.cpp ---===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-// this file is only relevant for Visual C++
-#if defined(_WIN32)
-
-#include "lldb/Host/windows/windows.h"
-
-#include "lldb/Host/windows/editlinewin.h"
-#include "llvm/Support/ErrorHandling.h"
-#include 
-#include 
-
-// edit line EL_ADDFN function pointer type
-typedef unsigned char (*el_addfn_func)(EditLine *e, int ch);
-typedef const char *(*el_prompt_func)(EditLine *);
-
-// edit line wrapper binding container
-struct el_binding {
-  //
-  const char *name;
-  const char *help;
-  // function pointer to callback routine
-  el_addfn_func func;
-  // ascii key this function is bound to
-  const char *key;
-};
-
-// stored key bindings
-static std::vector _bindings;
-
-// TODO: this should in fact be related to the exact edit line context we create
-static void *clientData = NULL;
-
-// store the current prompt string
-// default to what we expect to receive anyway
-static const char *_prompt = "(lldb) ";
-
-#if !defined(_WIP_INPUT_METHOD)
-
-static char *el_get_s(char *buffer, int chars) { return gets_s(buffer, chars); }
-#else
-
-static void con_output(char _in) {
-  HANDLE hout = GetStdHandle(STD_OUTPUT_HANDLE);
-  DWORD written = 0;
-  // get the cursor position
-  CONSOLE_SCREEN_BUFFER_INFO info;
-  GetConsoleScreenBufferInfo(hout, &info);
-  // output this char
-  WriteConsoleOutputCharacterA(hout, &_in, 1, info.dwCursorPosition, &written);
-  // advance cursor position
-  info.dwCursorPosition.X++;
-  SetConsoleCursorPosition(hout, info.dwCursorPosition);
-}
-
-static void con_backspace(void) {
-  HANDLE hout = GetStdHandle(STD_OUTPUT_HANDLE);
-  DWORD written = 0;
-  // get cursor position
-  CONSOLE_SCREEN_BUFFER_INFO info;
-  GetConsoleScreenBufferInfo(hout, &info);
-  // nudge cursor backwards
-  info.dwCursorPosition.X--;
-  SetConsoleCursorPosition(hout, info.dwCursorPosition);
-  // blank out the last character
-  WriteConsoleOutputCharacterA(hout, " ", 1, info.dwCursorPosition, &written);
-}
-
-static void con_return(void) {
-  HANDLE hout = GetStdHandle(STD_OUTPUT_HANDLE);
-  DWORD written = 0;
-  // get cursor position
-  CONSOLE_SCREEN_BUFFER_INFO info;
-  GetConsoleScreenBufferInfo(hout, &info);
-  // move onto the new line
-  info.dwCursorPosition.X = 0;
-  info.dwCursorPosition.Y++;
-  SetConsoleCursorPosition(hout, info.dwCursorPosition);
-}
-
-static bool runBind(char _key) {
-  for (int i = 0; i < _bindings.size(); i++) {
-el_binding *bind = _bindings[i];
-if (bind->key[0] == _key) {
-  bind->func((EditLine *)-1, _key);
-  return true;
-}
-  }
-  return false;
-}
-
-// replacement get_s which is EL_BIND aware
-static char *el_get_s(char *buffer, int chars) {
-  //
-  char *head = buffer;
-  //
-  for (;; Sleep(10)) {
-//
-INPUT_RECORD _record;
-//
-DWORD _read = 0;
-if (ReadConsoleInputA(GetStdHandle(STD_INPUT_HANDLE), &_record, 1,
-  &_read) == FALSE)
-  break;
-// if we didn't read a key
-if (_read == 0)
-  continue;
-// only interested in key events
-if (_record.EventType != KEY_EVENT)
-  continue;
-// is the key down
-if (!_record.Event.KeyEvent.bKeyDown)
-  continue;
-// read the ascii key character
-char _key = _record.Event.KeyEvent.uChar.AsciiChar;
-// non ascii conformant key press
-if (_key == 0) {
-  // check the scan code
-  // if VK_UP scroll back through history
-  // if VK_DOWN scroll forward through history
-  continue;
-}
-// try to execute any bind this key may have
-if (runBind(_key))
-  continue;
-// if we read a return key
-if (_key == '\n' || _key == '\r') {
-  con_return();
-  break;
-}
-// key is backspace
-if (_key == 0x8) {
-  // avoid deleting past beginning
-  if (head > buffer) {
-con_backspace();
-head--;
-  }
-  continue;
-}
-
-// add this key to the input buffer
-if ((head - buffer) < (chars - 1)) {
-  con_output(_key

[Lldb-commits] [PATCH] D102208: Remove Windows editline from LLDB

2021-05-21 Thread Neal via Phabricator via lldb-commits

nealsid added a comment.

I ran LLDB tests from a clean tree and with my change and the failures seemed 
to be the same, but there is a large amount of them.  I also did manual testing 
with the changes.  Build is still green.

Editline is only used when Cmake can find the editline libraries, which it 
never does on Windows.  So the code that is actually used for console I/O is 
the code in the IOHandlerEditline class in IOHandler.cpp that is outside of the 
#ifdef LLDB_USE_EDITLINE preprocessor check.  That uses POSIX functions for 
console I/O.  I think it may be a good follow up change to make 
IOHandlerEditline be used in editline-specific cases and move the 
fallback/non-editline code in that class to another class.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102208

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

[Lldb-commits] [PATCH] D102208: Remove Windows editline from LLDB

2021-05-21 Thread Stella Stamenova via Phabricator via lldb-commits

stella.stamenova added a comment.

I don't have context into this either. Does this cause any test failures or is 
everything still green after the change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102208

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

[Lldb-commits] [PATCH] D102208: Remove Windows editline from LLDB

2021-05-21 Thread Neal via Phabricator via lldb-commits

nealsid added a comment.

Thanks, all.  If there are no more comments, could someone land it for me? I 
don't have commit access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102208

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

[Lldb-commits] [PATCH] D102208: Remove Windows editline from LLDB

2021-05-21 Thread Adrian McCarthy via Phabricator via lldb-commits

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

LGTM.  Dead code should go.  If somebody wants to own a Windows port of 
EditLine, they can re-instate it and put some tests on it.

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102208

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

[Lldb-commits] [PATCH] D102208: Remove Windows editline from LLDB

2021-05-21 Thread David Spickett 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 rG5af3a6645f38: Remove Windows editline from LLDB (authored by 
nealsid, committed by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102208

Files:
  lldb/include/lldb/Host/Editline.h
  lldb/include/lldb/Host/windows/editlinewin.h
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/windows/EditLineWin.cpp

Index: lldb/source/Host/windows/EditLineWin.cpp
===
--- lldb/source/Host/windows/EditLineWin.cpp
+++ /dev/null
@@ -1,349 +0,0 @@
-//===-- EditLineWin.cpp ---===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-// this file is only relevant for Visual C++
-#if defined(_WIN32)
-
-#include "lldb/Host/windows/windows.h"
-
-#include "lldb/Host/windows/editlinewin.h"
-#include "llvm/Support/ErrorHandling.h"
-#include 
-#include 
-
-// edit line EL_ADDFN function pointer type
-typedef unsigned char (*el_addfn_func)(EditLine *e, int ch);
-typedef const char *(*el_prompt_func)(EditLine *);
-
-// edit line wrapper binding container
-struct el_binding {
-  //
-  const char *name;
-  const char *help;
-  // function pointer to callback routine
-  el_addfn_func func;
-  // ascii key this function is bound to
-  const char *key;
-};
-
-// stored key bindings
-static std::vector _bindings;
-
-// TODO: this should in fact be related to the exact edit line context we create
-static void *clientData = NULL;
-
-// store the current prompt string
-// default to what we expect to receive anyway
-static const char *_prompt = "(lldb) ";
-
-#if !defined(_WIP_INPUT_METHOD)
-
-static char *el_get_s(char *buffer, int chars) { return gets_s(buffer, chars); }
-#else
-
-static void con_output(char _in) {
-  HANDLE hout = GetStdHandle(STD_OUTPUT_HANDLE);
-  DWORD written = 0;
-  // get the cursor position
-  CONSOLE_SCREEN_BUFFER_INFO info;
-  GetConsoleScreenBufferInfo(hout, &info);
-  // output this char
-  WriteConsoleOutputCharacterA(hout, &_in, 1, info.dwCursorPosition, &written);
-  // advance cursor position
-  info.dwCursorPosition.X++;
-  SetConsoleCursorPosition(hout, info.dwCursorPosition);
-}
-
-static void con_backspace(void) {
-  HANDLE hout = GetStdHandle(STD_OUTPUT_HANDLE);
-  DWORD written = 0;
-  // get cursor position
-  CONSOLE_SCREEN_BUFFER_INFO info;
-  GetConsoleScreenBufferInfo(hout, &info);
-  // nudge cursor backwards
-  info.dwCursorPosition.X--;
-  SetConsoleCursorPosition(hout, info.dwCursorPosition);
-  // blank out the last character
-  WriteConsoleOutputCharacterA(hout, " ", 1, info.dwCursorPosition, &written);
-}
-
-static void con_return(void) {
-  HANDLE hout = GetStdHandle(STD_OUTPUT_HANDLE);
-  DWORD written = 0;
-  // get cursor position
-  CONSOLE_SCREEN_BUFFER_INFO info;
-  GetConsoleScreenBufferInfo(hout, &info);
-  // move onto the new line
-  info.dwCursorPosition.X = 0;
-  info.dwCursorPosition.Y++;
-  SetConsoleCursorPosition(hout, info.dwCursorPosition);
-}
-
-static bool runBind(char _key) {
-  for (int i = 0; i < _bindings.size(); i++) {
-el_binding *bind = _bindings[i];
-if (bind->key[0] == _key) {
-  bind->func((EditLine *)-1, _key);
-  return true;
-}
-  }
-  return false;
-}
-
-// replacement get_s which is EL_BIND aware
-static char *el_get_s(char *buffer, int chars) {
-  //
-  char *head = buffer;
-  //
-  for (;; Sleep(10)) {
-//
-INPUT_RECORD _record;
-//
-DWORD _read = 0;
-if (ReadConsoleInputA(GetStdHandle(STD_INPUT_HANDLE), &_record, 1,
-  &_read) == FALSE)
-  break;
-// if we didn't read a key
-if (_read == 0)
-  continue;
-// only interested in key events
-if (_record.EventType != KEY_EVENT)
-  continue;
-// is the key down
-if (!_record.Event.KeyEvent.bKeyDown)
-  continue;
-// read the ascii key character
-char _key = _record.Event.KeyEvent.uChar.AsciiChar;
-// non ascii conformant key press
-if (_key == 0) {
-  // check the scan code
-  // if VK_UP scroll back through history
-  // if VK_DOWN scroll forward through history
-  continue;
-}
-// try to execute any bind this key may have
-if (runBind(_key))
-  continue;
-// if we read a return key
-if (_key == '\n' || _key == '\r') {
-  con_return();
-  break;
-}
-// key is backspace
-if (_key == 0x8) {
-  // avoid deleting past beginning
-  if (head > buffer) {
-con_backspace();
-head--;
-  }
-  continue;
-}
-
-

38 matches

Mail list logo