djtodoro added a comment.
Thanks a lot for this!
> Nevertheless, I am still interested in making assembly-based tests for this
> (and similar features) because it enables testing scenarios that we could not
> get (reliably or at all) a compiler to produce.
I also think this would be more stable if we can make assembler-based tests
(but we'll need to address all archs from {x86_64, arm, aarch64}).
I am just wondering, what are the obstacles for writing the assembler-based
tests? Is it LLDB testing infrastructure or writing tests itself?
================
Comment at:
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp:21-22
+ ++global;
//% self.filecheck("image lookup -va $pc", "main.cpp",
"-check-prefix=FUNC1-DESC")
// FUNC1-DESC: name = "sink", type = "int &", location =
DW_OP_entry_value(DW_OP_reg5 RDI)
}
----------------
labath wrote:
> vsk wrote:
> > labath wrote:
> > > If we remove this check, the test will be completely architecture- and
> > > abi-independent. I don't think this check is particularly useful (we use
> > > llvm to print the dwarf expression, and there are better ways to test the
> > > image lookup command). Maybe we could just keep it to ensure that we
> > > really are evaluating entry values, but change the check the just search
> > > for the DW_OP_entry_value keyword (and then run the test on all
> > > architectures)?
> > We should stop matching %rdi, as the purpose of the check is just to
> > determine whether we really are testing entry value evaluation. However,
> > llvm doesn't support entry value production for all platforms, so we would
> > need to restrict the test to {x86_64, arm, aarch64} (still a clear
> > improvement over the current situation).
> Sounds good. (I'm not sure we even have functioning bots for non-x86, non-arm
> platforms).
>We should stop matching %rdi, as the purpose of the check is just to determine
>whether we really are testing entry value evaluation. However, llvm doesn't
>support entry value production for all platforms, so we would need to restrict
>the test to {x86_64, arm, aarch64} (still a clear improvement over the current
>situation).
+1
We can teach the `TestBasicEntryValuesX86_64.py` to keep arm and aarch64 as
well.
Furthermore, we can add a function into decorators something like
`skipUnlessEntryValuesSupportedArch` checking if the arch is in {x86_64, arm,
aarch64}.
================
Comment at:
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp:61-62
-
- //% self.filecheck("expr x", "main.cpp", "-check-prefix=FUNC2-EXPR-FAIL",
expect_cmd_failure=True)
- // FUNC2-EXPR-FAIL: couldn't get the value of variable x: variable not
available
-
----------------
vsk wrote:
> labath wrote:
> > I'm not sure why this was previously expected to fail -- I don't see a
> > reason why the compiler couldn't emit an entry value for `x` now, or before
> > this patch. And in the new setup, the expression actually succeeds.
> IIRC this is left over from when entry value propagation in LiveDebugValues
> was being reworked.
Yes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79491/new/
https://reviews.llvm.org/D79491
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
