This is an archive of the discontinued LLVM Phabricator instance.

[OMPT] Fix test cancel_parallel.c
ClosedPublic

Authored by Hahnfeld on Nov 6 2017, 5:29 PM.

Details

Summary

If a parallel region is cancelled, execution resumes at the end
of the structured block. That is why this test cannot use the
"normal" macros that print right after inserting the label.
Instead it previously printed the addresses before the pragma
and swapped the checks compared to the other tests.

However, this does not work because FileChecks '*' is greedy
so that RETURN_ADDRESS always matched the second address. This
makes the test fail when an "overflow" occurrs and the first
address matches the value of codeptr_ra.

I discovered this on my MacBook but I'm unable to reproduce the
failure with the current version. Nevertheless we should fix this
problem to avoid that this test fails later after an unrelated change.

Diff Detail

Repository
rL LLVM

Event Timeline

Hahnfeld created this revision.Nov 6 2017, 5:29 PM
Hahnfeld edited subscribers, added: openmp-commits; removed: cfe-commits.
protze.joachim edited edge metadata.Nov 8 2017, 9:19 AM

Instead of saving the address and printing it in a new team afterwards, you could also use CHECK-DAG and print the address, where you store it.
In that case, you should add another, unrelated match in between (e.g., the thread_begin event)

Not sure, which approach is better :)

Hahnfeld updated this revision to Diff 122119.Nov 8 2017, 10:54 AM

Use CHECK-DAG proposed by Joachim

Instead of saving the address and printing it in a new team afterwards, you could also use CHECK-DAG and print the address, where you store it.
In that case, you should add another, unrelated match in between (e.g., the thread_begin event)

Not sure, which approach is better :)

This one! I honestly wasn't aware that CHECK-DAG can still match variables, I thought that wouldn't work. Documentation and tests suggest it works

This revision is now accepted and ready to land.Nov 9 2017, 5:26 AM
This revision was automatically updated to reflect the committed changes.