vsk added a comment.

Thanks for working on this!



================
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:
> 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).


================
Comment at: 
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp:34
+  // FUNC2-EXPR1: ${{.*}} = 123
+  // FUNC2-EXPR2: ${{.*}} = 2
 }
----------------
Hm, I thought inline tests only supported running one command per breakpoint. 
Have you tried running this test with `--param dotest-args='-t'` to verify that 
FUNC2-EXPR2 gets matched? If it does, that's great; if not, we can manufacture 
a second breakpoint by adding another '++global' or convert this to an API test.


================
Comment at: 
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp:56
+  //%     expect_cmd_failure=True)
+  // FUNC4-EXPR-FAIL: couldn't get the value of variable x: Could not evaluate 
DW_OP_entry_value.
+  // FUNC4-EXPR: couldn't get the value of variable sink:
----------------
Ditto, I have the same concern about whether 'expr sink' is evaluated.


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


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

Reply via email to