Repository navigation
Conversation
…sk, or count expr2tags_Unop_DC now takes the operation. Unary operations whose result is a boolean, or a mask or count computed from the operand's bits (CmpNEZ*, CmpwNEZ*, 32to1, 64to1, Clz*, Ctz*, PopCount*, GetMSBs*), give the result tag 0, as binary comparisons do. All other unary operations return the tag of the operand, as before. The operand's tag is passed to MC_(helperc_RETURN_0) rather than discarded. Returning a constant tag would let VEX remove the operand's tag expression as dead code, along with any merge it performed. For example, in 'char lt = (a < b)' the merge of a and b would be lost.
The new option controls what happens when DynComp reaches the default case for a binary, ternary, or quaternary VEX operation that it does not handle. "panic" (the default) stops, as before. "warn" prints a warning once per operation and approximates the operation by merging its operands' tags.
expr2tags_Unop_DC now lists every unary operation, like the binary, ternary, and quaternary functions do, and an operation that DynComp does not handle reaches the default case. That case uses report_unhandled_op_DC, so --dyncomp-unhandled-ops=warn approximates an unhandled unary operation by returning its operand's tag.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Change: Bug fix Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 61.90% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 7 files. (2 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 61.90% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 7 files. (2 skipped: 2 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
…perands Comparisons merge their operands' tags with a dirty call, so VEX cannot remove the merge when a later operation discards the comparison's tag. Unary operations that give tag 0 anchor their operand's tag with a dirty MC_(helperc_TAG_NOP) call (except in the dataflow modes) and return constant 0, replacing MC_(helperc_RETURN_0). Iop_32to1 and Iop_64to1 return the operand's tag again, because they also extract data bits. Iop_Cnt* and Iop_Cls* give tag 0. Add a regression test, dyncomp-unop-tag0-test.sh.
In warn mode, do not merge operands that appear to be rounding modes, shift amounts, or lane indices; treat boolean-valued ops as comparisons; and treat other ops as non-interactions in units and comparisons modes. Also realign and clarify the --help text.
|
@coderabbitai review |
✅ Action performedReview finished.
|
…omp-unop-all-cases
…unop-all-cases # Conflicts: # valgrind/fjalar/kvasir/dyncomp_translate.c
…unary operation codespecs#85 replaced report_unhandled_op_DC by approximate_unhandled_op_DC, which handled only binary, ternary, and quaternary operations. An unhandled unary operation still returns the tag of its operand.
This branch has not been deployed
1 participant
This pull request depends on #85 and #86, and it contains their commits. Please merge #85 and #86 first. After that, this pull request's diff shows only its own change.
Before this pull request, the list of unary operations in
expr2tags_Unop_DCwas only a comment. The binary, ternary, and quaternary functions instead switch on the operation and handle an operation that they do not know in thedefaultcase. This pull request makes the unary function consistent with them. It covers all 283 unary operations inVEX/priv/ir_defs.c(typeOfPrimop):Not,Neg, conversions,32to1,64to1, ...).The
defaultcase callsapproximate_unhandled_op_DC(from #85), which this pull request extends to unary operations. With--dyncomp-unhandled-ops=warn, an unhandled unary operation returns its operand's tag.The comment list contained two binary operations (
Iop_MulHi8Sx16andIop_MulHi8Ux16). They are not unary cases.This is one of three pull requests split from #83.