[Lldb-commits] [PATCH] D61611: [JITLoaderGDB] Set eTypeJIT for objects read from JIT descriptors

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

In D61611#1496580 <https://reviews.llvm.org/D61611#1496580>, @sgraenitz wrote:

> @stella.stamenova Can you have a look at the lit test please? It works on macOS and Linux, but I didn't test Windows. Should I add something like `# REQUIRES: nowindows` or is it fine like this?


Maybe I'm pessimistic, but I'd guess that this will fail on windows because some of the features this needs is not implemented there (but I'm not sure which one). You can just add `XFAIL: system-windows` and then you'll get an email if the guess is wrong.



================
Comment at: lldb/lit/Breakpoint/jitbp_elf.test:1
+# REQUIRES: target-x86_64, system-linux, native
+
----------------
sgraenitz wrote:
> labath wrote:
> > sgraenitz wrote:
> > > The test only works with ELF on Linux. Is the `REQUIRES` sufficient?
> > Yes, but what is the reason for that? It looks like the test is generic enough that it should run on any system where lli is able to jit code. In particular I'd expect this to also work on macOS if you set `plugin-jit-loader.gdb.enable` to `on`.
> Indeed, since https://reviews.llvm.org/D57689 it does work on macOS with ELF.
Cool. I think you can also drop the `target-x86_64` part, as I don't see a reason why this shouldn't work on arm for instance (though we don't have any arm bots around to verify that). I am not sure why you're using the `--target` argument to clang -- I think you should be able to just drop that and make clang generate IR for the host. (the `native` feature ensures that "host" is the default target for clang).


================
Comment at: lldb/lit/Breakpoint/jitbp_elf.test:3
+
+# RUN: %clang -g -S -emit-llvm -o %t.ll %p/Inputs/jitbp.cpp
+# RUN: %lldb -b -o 'b jitbp' -o 'run -jit-kind=mcjit %t.ll' lli | FileCheck %s
----------------
sgraenitz wrote:
> labath wrote:
> > sgraenitz wrote:
> > > With these args, clang shouldn't optimize away `jitbp()`
> > Is that a question? If it is I'm pretty sure the answer is "it shouldn't", because a lot of our tests depend on -O0 not doing anything funny.
> Yes, I didn't know whether `-g` is enough or I better pass something like `-O0` explicitly?
I'm pretty sure -O0 will always be the default opt level for clang (and -g as a matter of principle has no impact on codegen decisions like inlining).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61611