llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-backend-risc-v Author: None (dlav-sc) <details> <summary>Changes</summary> Currently, the tests that check stepping through atomic sequences use a hardcoded step distance, which is unreliable because this distance depends on LLVM's codegeneration. This patch rewrites the test to avoid this disadvantage. The test now checks the opcode of the instruction after the step instead of the step distance. --- Full diff: https://github.com/llvm/llvm-project/pull/156506.diff 3 Files Affected: - (modified) lldb/test/API/riscv/step/TestSoftwareStep.py (+21-14) - (modified) lldb/test/API/riscv/step/branch.c (+1) - (modified) lldb/test/API/riscv/step/main.c (+2-1) ``````````diff diff --git a/lldb/test/API/riscv/step/TestSoftwareStep.py b/lldb/test/API/riscv/step/TestSoftwareStep.py index 279c4c1b797e0..0544d2102d0f9 100644 --- a/lldb/test/API/riscv/step/TestSoftwareStep.py +++ b/lldb/test/API/riscv/step/TestSoftwareStep.py @@ -26,19 +26,26 @@ def do_sequence_test(self, filename, bkpt_name): substrs=["stopped", "stop reason = instruction step into"], ) - pc = cur_thread.GetFrameAtIndex(0).GetPC() + # Get the instruction we stopped at + pc = cur_thread.GetFrameAtIndex(0).GetPCAddress() + inst = target.ReadInstructions(pc, 1).GetInstructionAtIndex(0) - return pc - entry_pc + inst_mnemonic = inst.GetMnemonic(target) + inst_operands = inst.GetOperands(target) + if not inst_operands: + return inst_mnemonic - @skipIf(archs=no_match("^rv.*")) + return f"{inst_mnemonic} {inst_operands}" + + @skipIf(archs=no_match("^riscv.*")) def test_cas(self): """ This test verifies LLDB instruction step handling of a proper lr/sc pair. """ - difference = self.do_sequence_test("main", "cas") - self.assertEqual(difference, 0x1A) + instruction = self.do_sequence_test("main", "cas") + self.assertEqual(instruction, "nop") - @skipIf(archs=no_match("^rv.*")) + @skipIf(archs=no_match("^riscv.*")) def test_branch_cas(self): """ LLDB cannot predict the actual state of registers within a critical section (i.e., inside an atomic @@ -51,29 +58,29 @@ def test_branch_cas(self): test is nearly identical to the previous one, except for the branch condition, which is inverted and will result in a taken jump. """ - difference = self.do_sequence_test("branch", "branch_cas") - self.assertEqual(difference, 0x1A) + instruction = self.do_sequence_test("branch", "branch_cas") + self.assertEqual(instruction, "ret") - @skipIf(archs=no_match("^rv.*")) + @skipIf(archs=no_match("^riscv.*")) def test_incomplete_sequence_without_lr(self): """ This test verifies the behavior of a standalone sc instruction without a preceding lr. Since the sc lacks the required lr pairing, LLDB should treat it as a non-atomic store rather than part of an atomic sequence. """ - difference = self.do_sequence_test( + instruction = self.do_sequence_test( "incomplete_sequence_without_lr", "incomplete_cas" ) - self.assertEqual(difference, 0x4) + self.assertEqual(instruction, "and a5, a2, a4") - @skipIf(archs=no_match("^rv.*")) + @skipIf(archs=no_match("^riscv.*")) def test_incomplete_sequence_without_sc(self): """ This test checks the behavior of a standalone lr instruction without a subsequent sc. Since the lr lacks its required sc counterpart, LLDB should treat it as a non-atomic load rather than part of an atomic sequence. """ - difference = self.do_sequence_test( + instruction = self.do_sequence_test( "incomplete_sequence_without_sc", "incomplete_cas" ) - self.assertEqual(difference, 0x4) + self.assertEqual(instruction, "and a5, a2, a4") diff --git a/lldb/test/API/riscv/step/branch.c b/lldb/test/API/riscv/step/branch.c index 79bf0ca005db1..93d6c51ec75e0 100644 --- a/lldb/test/API/riscv/step/branch.c +++ b/lldb/test/API/riscv/step/branch.c @@ -11,6 +11,7 @@ void __attribute__((naked)) branch_cas(int *a, int *b) { "xor a5, a2, a5\n\t" "sc.w a5, a1, (a3)\n\t" "beqz a5, 1b\n\t" + "nop\n\t" "2:\n\t" "ret\n\t"); } diff --git a/lldb/test/API/riscv/step/main.c b/lldb/test/API/riscv/step/main.c index 35e8aee2cae4b..6207954b7e1cb 100644 --- a/lldb/test/API/riscv/step/main.c +++ b/lldb/test/API/riscv/step/main.c @@ -1,5 +1,5 @@ void __attribute__((naked)) cas(int *a, int *b) { - // This atomic sequence implements a copy-and-swap function. This test should + // This atomic sequence implements a copy-and-swap function. This test should stop // at the first instruction, and after step instruction, we should stop at the // end of the sequence (on the ret instruction). asm volatile("1:\n\t" @@ -11,6 +11,7 @@ void __attribute__((naked)) cas(int *a, int *b) { "xor a5, a2, a5\n\t" "sc.w a5, a1, (a3)\n\t" "beqz a5, 1b\n\t" + "nop\n\t" "2:\n\t" "ret\n\t"); } `````````` </details> https://github.com/llvm/llvm-project/pull/156506 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
