On Mon, Nov 12, 2018 at 07:52:28PM +0100, Philippe Mathieu-Daudé wrote: > On Mon, Nov 12, 2018 at 6:03 PM Eduardo Habkost <ehabk...@redhat.com> wrote: > > On Mon, Nov 12, 2018 at 12:36:16AM +0100, Philippe Mathieu-Daudé wrote: > > > Python internal representation is signed, so unsigned values > > > bigger than 31-bit are interpreted as signed (and printed with > > > a '-' signed). > > > Mask out to force unsigned values. > > > > I don't understand this commit description. Python surely > > supports integers larger than 2^31, and its internal > > representation shouldn't matter at all: > > > > >>> > > '0x{0:08x}'.format(0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff) > > > > '0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff' > > > > Can you explain how the code ends up with a negative value in the > > `self.thismask` or `b` variables? If `self.subs` contains > > negative values, this is likely to break other parts of the code. > > I guess I misunderstood the error, thus the description is invalid. > > > > > > > > > Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org> > > > --- > > > TODO: display error encountered: > > > > > > case 0x-1: > > > .... > > > > How can I reproduce it? > > $ scripts/decodetree.py /dev/null > > switch (insn & 0x-0000001) { > } > > main() -> > build_tree(patterns, 0, outermask = 0) -> > innermask = ~outermask # = -1 > Tree(fullmask, innermask = -1) -> > __init__(self, fm, tm = -1) -> > self.thismask = tm # -1 > t.output_code(4, False, 0, 0) -> > sh = is_contiguous(self.thismask) # -1 > output(ind, 'switch (', str_switch(self.thismask), ') {\n') > > with: > > def str_switch(b): > return 'insn & 0x{0:08x}'.format(b) > > So the fix is rather: > > -- >8 -- > diff --git a/scripts/decodetree.py b/scripts/decodetree.py > @@ -916,7 +916,7 @@ class Tree: > > def build_tree(pats, outerbits, outermask): > # Find the intersection of all remaining fixedmask. > - innermask = ~outermask > + innermask = ~outermask & insnmask > for i in pats: > innermask &= i.fixedmask
Nice. Thanks for the explanation. This really seems to be the only place in the code where the bit operations may generate a negative number. All remaining usage of the ~ operator in the code is already inside & expressions that will ensure the result is positive. To the alternative fix above: Reviewed-by: Eduardo Habkost <ehabk...@redhat.com> -- Eduardo