gdb.rocm/watchpoint-basic: add KFAILs for known configurations#203
gdb.rocm/watchpoint-basic: add KFAILs for known configurations#203amd-shahab wants to merge 1 commit into
Conversation
Some of the tests in gdb.rom/watchpoint-basic are destined to fail due to a known failure in KFD. This patch marks those tests as such on configurations that this can happen. Change-Id: Iafd40b18bf8eb7004c8653bca1407b8d86ca0032
| "Hardware watchpoint $::decimal: .*" \ | ||
| "set watchpoint on *ptr1" | ||
|
|
||
| maybe_kfail |
There was a problem hiding this comment.
Couldn't we stop at the first failure and bail out? We'd have the KFAIL marked and we could carry on with other tests. I'm not sure I see the value in the multiple KFAIL's unless each of them is unique and may randomly happen or may not happen.
There was a problem hiding this comment.
At the first sight, this would cause inconsistent number of tests between different systems.
However, please add a patch snippet of what you propose, so I can understand it better.
There was a problem hiding this comment.
It would cause the number of tests to differ indeed. But if the tests we're potentially marking as KFAIL are of the same type (waiting for a watchpoint hit), I think that would be OK. For me a single KFAIL would already convey the message that there is something broken beyond our control.
My suggestion was to detect early when a target is misbehaving in a known way, catch that, issue a KFAIL and then stop the test. Just a suggestion anyway.
It also avoids polluting the test with a lot of calls to maybe_kfail.
There was a problem hiding this comment.
having that many maybe_kfail feels wrong. It makes the test unreadable.
We do have other cases where we bail out early, for example it is quite common to have
if {![runto_main]} {
return
}which would also change the number of test results.
If we want a kfail each time, we would at least include this in the definition of continue_to_watchpoint_hit, not at every call, and have a similar helper to do the continue.
I do not really know if upstream has a policy regarding this, comments welcome.
What we need is to be sure to execute enough steps in the test to actually see the failure (or new success when we will eventually have a fix, this will become a kpass), so this is not ignored.
| } | ||
| } | ||
|
|
||
| # On some systems, KFD messes up the watchpoint configurations |
There was a problem hiding this comment.
I find the "messes up" language a bit strong here. Might be worth softening it a bit.
| gdb_caching_proc target_has_kfail {} { | ||
| set kfail_arches {gfx1201} | ||
|
|
||
| # Check for targets where this test will fail without precise memory. |
There was a problem hiding this comment.
is precise memory involved here? I do not expect so.
|
|
||
| proc maybe_kfail {} { | ||
| if {[target_has_kfail]} { | ||
| setup_kfail "*-*-*" "bad queue mapping in kfd." |
There was a problem hiding this comment.
Queue mapping works, watchpoint configuration does not.
| "Hardware watchpoint $::decimal: .*" \ | ||
| "set watchpoint on *ptr1" | ||
|
|
||
| maybe_kfail |
There was a problem hiding this comment.
having that many maybe_kfail feels wrong. It makes the test unreadable.
We do have other cases where we bail out early, for example it is quite common to have
if {![runto_main]} {
return
}which would also change the number of test results.
If we want a kfail each time, we would at least include this in the definition of continue_to_watchpoint_hit, not at every call, and have a similar helper to do the continue.
I do not really know if upstream has a policy regarding this, comments welcome.
What we need is to be sure to execute enough steps in the test to actually see the failure (or new success when we will eventually have a fix, this will become a kpass), so this is not ignored.
Pedantically, upstream, we use KFAIL for known issues in GDB itself (that we're too lazy^W^W^W we don't have the time to fix right now), and XFAIL for known external problems we don't have control over. This sounds like the latter kind. This helps when looking at gdb.sum results, as KFAILs are things we can work on to fix, XFAILs, not so much. |
4 participants
Motivation
Failures on GFX1201.
Technical Details
Some of the tests in gdb.rom/watchpoint-basic are destined to fail due to a known failure in KFD. This patch marks those tests as such on configurations that this can happen. See AIROCGDB-555.
Test Plan
See the failing tests as KFAILs on a GFX1201 system.
Test Result