| Message ID | 20200518164052.18689-6-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | decodetree: Add non-overlapping groups | expand |
On Mon, 18 May 2020 at 17:41, Richard Henderson <richard.henderson@linaro.org> wrote: > > This is an edge case for sure, but the logic that disallowed > this case was faulty. Further, a few fixes scattered about > can allow this to work. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > ...est1.decode => succ_pattern_group_nest2.decode} | 2 +- > scripts/decodetree.py | 14 +++++++++++--- > 2 files changed, 12 insertions(+), 4 deletions(-) > rename tests/decode/{err_pattern_group_nest1.decode => succ_pattern_group_nest2.decode} (85%) > @@ -978,6 +980,12 @@ def build_tree(pats, outerbits, outermask): > innermask &= i.fixedmask > > if innermask == 0: > + # Edge condition: One pattern covers the entire insnmask > + if len(pats) == 1: > + t = Tree(outermask, innermask) > + t.subs.append((0, pats[0])) > + return t > + > text = 'overlapping patterns:' > for p in pats: > text += '\n' + p.file + ':' + str(p.lineno) + ': ' + str(p) I don't really understand this code, but does the similar looking build_size_tree() also need a change to handle a length-one pats ? thanks -- PMM
On 6/2/20 7:35 AM, Peter Maydell wrote: > On Mon, 18 May 2020 at 17:41, Richard Henderson > <richard.henderson@linaro.org> wrote: >> >> This is an edge case for sure, but the logic that disallowed >> this case was faulty. Further, a few fixes scattered about >> can allow this to work. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> ...est1.decode => succ_pattern_group_nest2.decode} | 2 +- >> scripts/decodetree.py | 14 +++++++++++--- >> 2 files changed, 12 insertions(+), 4 deletions(-) >> rename tests/decode/{err_pattern_group_nest1.decode => succ_pattern_group_nest2.decode} (85%) > >> @@ -978,6 +980,12 @@ def build_tree(pats, outerbits, outermask): >> innermask &= i.fixedmask >> >> if innermask == 0: >> + # Edge condition: One pattern covers the entire insnmask >> + if len(pats) == 1: >> + t = Tree(outermask, innermask) >> + t.subs.append((0, pats[0])) >> + return t >> + >> text = 'overlapping patterns:' >> for p in pats: >> text += '\n' + p.file + ':' + str(p.lineno) + ': ' + str(p) > > I don't really understand this code, but does the similar > looking build_size_tree() also need a change to handle a > length-one pats ? I don't think so, because in that case we'd exit earlier with if onewidth: return SizeLeaf(innermask, minwidth) r~
On Mon, 18 May 2020 at 17:41, Richard Henderson <richard.henderson@linaro.org> wrote: > > This is an edge case for sure, but the logic that disallowed > this case was faulty. Further, a few fixes scattered about > can allow this to work. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > -- Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
diff --git a/tests/decode/err_pattern_group_nest1.decode b/tests/decode/succ_pattern_group_nest2.decode similarity index 85% rename from tests/decode/err_pattern_group_nest1.decode rename to tests/decode/succ_pattern_group_nest2.decode index 92e971c3c5..8d5ab4b2d3 100644 --- a/tests/decode/err_pattern_group_nest1.decode +++ b/tests/decode/succ_pattern_group_nest2.decode @@ -6,7 +6,7 @@ %sub3 16:8 %sub4 24:8 -# Groups with no overlap are supposed to fail +# Group with complete overlap of the two patterns { top 00000000 00000000 00000000 00000000 sub4 ........ ........ ........ ........ %sub1 %sub2 %sub3 %sub4 diff --git a/scripts/decodetree.py b/scripts/decodetree.py index ea313bcdea..3307c74c30 100755 --- a/scripts/decodetree.py +++ b/scripts/decodetree.py @@ -124,6 +124,7 @@ def is_pow2(x): def ctz(x): """Return the number of times 2 factors into X.""" + assert x != 0 r = 0 while ((x >> r) & 1) == 0: r += 1 @@ -131,6 +132,8 @@ def ctz(x): def is_contiguous(bits): + if bits == 0: + return -1 shift = ctz(bits) if is_pow2((bits >> shift) + 1): return shift @@ -793,9 +796,8 @@ def build_incmulti_pattern(lineno, pats): error(lineno, 'width mismatch in patterns within braces') repeat = True - while repeat: - if fixedmask == 0: - error(lineno, 'no overlap in patterns within braces') + fixedbits = 0 + while repeat and fixedmask != 0: fixedbits = None for p in pats: thisbits = p.fixedbits & fixedmask @@ -978,6 +980,12 @@ def build_tree(pats, outerbits, outermask): innermask &= i.fixedmask if innermask == 0: + # Edge condition: One pattern covers the entire insnmask + if len(pats) == 1: + t = Tree(outermask, innermask) + t.subs.append((0, pats[0])) + return t + text = 'overlapping patterns:' for p in pats: text += '\n' + p.file + ':' + str(p.lineno) + ': ' + str(p)
This is an edge case for sure, but the logic that disallowed this case was faulty. Further, a few fixes scattered about can allow this to work. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- ...est1.decode => succ_pattern_group_nest2.decode} | 2 +- scripts/decodetree.py | 14 +++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) rename tests/decode/{err_pattern_group_nest1.decode => succ_pattern_group_nest2.decode} (85%) -- 2.20.1