Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MC/DC] Introduce class TestVector with a pair of BitVector #82174

Merged
merged 7 commits into from
Feb 27, 2024

Conversation

chapuni
Copy link
Contributor

@chapuni chapuni commented Feb 18, 2024

This replaces SmallVector<CondState> and emulates it.

  • -------- True False DontCare
  • Values: True False False
  • Visited: True True False

findIndependencePairs() can be optimized with logical ops.

FIXME: Specialize findIndependencePairs() for the single word.

This replaces `SmallVector<CondState>` and emulates it.

-          True  False DontCare (Impossible)
- Values:  True  False False    True
- Visited: True  True  False    False

`findIndependencePairs()` can be optimized with logical ops.

FIXME: Specialize `findIndependencePairs()` for the single word.
BitVector Visited; /// ~DontCare

public:
/// Assume filling DontCare.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need to pass Cond when it is not being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because the only user (TV in findExecutedTestVectors()) specifies DontCare explicitly. I wanted to leave it since I thought it was more descriptive.

I can prune the initial value in this.

}

/// Emulate RHS SmallVector::operator[]
MCDCRecord::CondState operator[](int I) const {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you expand on this comment?

E.g.

For the condition at index \p I:
\returns MCDC_True when the condition has been visited
\returns MCDC_False when a condition has not been visited
\returns MCDC_DontCare when the condition does not impact MC/DC coverage

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they might be redundant to explain here since this handles the enum.

/// Equivalent to buildTestVector's Index.
auto getIndex() const { return Values.getData()[0]; }

/// Emulate LHS SmallVector::operator[].
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment could be like

Set the condition at position \p I's CondState to \p Val.

Copy link

@ornata ornata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the code looks fine. I left a couple nits on comments. But otherwise, LGTM.

Copy link
Contributor Author

@chapuni chapuni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to wait for both @MaskRay and @evodius96 , since they are authors of the current impl.

BitVector Visited; /// ~DontCare

public:
/// Assume filling DontCare.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because the only user (TV in findExecutedTestVectors()) specifies DontCare explicitly. I wanted to leave it since I thought it was more descriptive.

I can prune the initial value in this.

}

/// Emulate RHS SmallVector::operator[]
MCDCRecord::CondState operator[](int I) const {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they might be redundant to explain here since this handles the enum.

Copy link
Contributor Author

@chapuni chapuni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cost in the loop could be reduced more to adopt also #82282.

Comment on lines 321 to 322
if (!A.isDifferentOutcome(B))
continue;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be eliminated.

IndependencePairs.insert({Flip, std::make_pair(J + 1, I + 1)});
auto AB = A.getDifferences(B);
assert(AB[NumConditions] && "The last element should be different");
if (AB.count() == 2) // The single condition and the last element
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be == 1 aka has_single_bit.

@chapuni chapuni merged commit 512a8a7 into llvm:main Feb 27, 2024
3 of 4 checks passed
@chapuni chapuni deleted the mcdc/refactor/tvbv branch February 27, 2024 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants