* [PATCH v3 01/30] decodetree: Introduce whex and whexC helpers
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 13:01 ` Luis Fernando Fujita Pires
2021-05-03 22:32 ` Philippe Mathieu-Daudé
2021-04-30 1:15 ` [PATCH v3 02/30] decodetree: More use of f-strings Richard Henderson
` (29 subsequent siblings)
30 siblings, 2 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Form a hex constant of the appropriate insnwidth.
Begin using f-strings on changed lines.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
scripts/decodetree.py | 66 +++++++++++++++++++++++++------------------
1 file changed, 38 insertions(+), 28 deletions(-)
diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 4637b633e7..0861e5d503 100644
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -102,6 +102,21 @@ def str_fields(fields):
return r[1:]
+def whex(val):
+ """Return a hex string for val padded for insnwidth"""
+ global insnwidth
+ return f'0x{val:0{insnwidth // 4}x}'
+
+
+def whexC(val):
+ """Return a hex string for val padded for insnwidth,
+ and with the proper suffix for a C constant."""
+ suffix = ''
+ if val >= 0x80000000:
+ suffix = 'u'
+ return whex(val) + suffix
+
+
def str_match_bits(bits, mask):
"""Return a string pretty-printing BITS/MASK"""
global insnwidth
@@ -477,11 +492,8 @@ def output_code(self, i, extracted, outerbits, outermask):
if outermask != p.fixedmask:
innermask = p.fixedmask & ~outermask
innerbits = p.fixedbits & ~outermask
- output(ind, 'if ((insn & ',
- '0x{0:08x}) == 0x{1:08x}'.format(innermask, innerbits),
- ') {\n')
- output(ind, ' /* ',
- str_match_bits(p.fixedbits, p.fixedmask), ' */\n')
+ output(ind, f'if ((insn & {whexC(innermask)}) == {whexC(innerbits)}) {{\n')
+ output(ind, f' /* {str_match_bits(p.fixedbits, p.fixedmask)} */\n')
p.output_code(i + 4, extracted, p.fixedbits, p.fixedmask)
output(ind, '}\n')
else:
@@ -500,12 +512,12 @@ def __init__(self, fm, tm):
def str1(self, i):
ind = str_indent(i)
- r = '{0}{1:08x}'.format(ind, self.fixedmask)
+ r = ind + whex(self.fixedmask)
if self.format:
r += ' ' + self.format.name
r += ' [\n'
for (b, s) in self.subs:
- r += '{0} {1:08x}:\n'.format(ind, b)
+ r += ind + f' {whex(b)}:\n'
r += s.str1(i + 4) + '\n'
r += ind + ']'
return r
@@ -529,16 +541,16 @@ def output_code(self, i, extracted, outerbits, outermask):
if sh > 0:
# Propagate SH down into the local functions.
def str_switch(b, sh=sh):
- return '(insn >> {0}) & 0x{1:x}'.format(sh, b >> sh)
+ return f'(insn >> {sh}) & {b >> sh:#x}'
def str_case(b, sh=sh):
- return '0x{0:x}'.format(b >> sh)
+ return hex(b >> sh)
else:
def str_switch(b):
- return 'insn & 0x{0:08x}'.format(b)
+ return f'insn & {whexC(b)}'
def str_case(b):
- return '0x{0:08x}'.format(b)
+ return whexC(b)
output(ind, 'switch (', str_switch(self.thismask), ') {\n')
for b, s in sorted(self.subs):
@@ -962,19 +974,19 @@ def parse_generic(lineno, parent_pat, name, toks):
# Validate the masks that we have assembled.
if fieldmask & fixedmask:
- error(lineno, 'fieldmask overlaps fixedmask (0x{0:08x} & 0x{1:08x})'
- .format(fieldmask, fixedmask))
+ error(lineno, 'fieldmask overlaps fixedmask ',
+ f'({whex(fieldmask)} & {whex(fixedmask)})')
if fieldmask & undefmask:
- error(lineno, 'fieldmask overlaps undefmask (0x{0:08x} & 0x{1:08x})'
- .format(fieldmask, undefmask))
+ error(lineno, 'fieldmask overlaps undefmask ',
+ f'({whex(fieldmask)} & {whex(undefmask)})')
if fixedmask & undefmask:
- error(lineno, 'fixedmask overlaps undefmask (0x{0:08x} & 0x{1:08x})'
- .format(fixedmask, undefmask))
+ error(lineno, 'fixedmask overlaps undefmask ',
+ f'({whex(fixedmask)} & {whex(undefmask)})')
if not is_format:
allbits = fieldmask | fixedmask | undefmask
if allbits != insnmask:
- error(lineno, 'bits left unspecified (0x{0:08x})'
- .format(allbits ^ insnmask))
+ error(lineno, 'bits left unspecified ',
+ f'({whex(allbits ^ insnmask)})')
# end parse_general
@@ -1104,10 +1116,9 @@ def __init__(self, m, w):
def str1(self, i):
ind = str_indent(i)
- r = '{0}{1:08x}'.format(ind, self.mask)
- r += ' [\n'
+ r = ind + whex(self.mask) + ' [\n'
for (b, s) in self.subs:
- r += '{0} {1:08x}:\n'.format(ind, b)
+ r += ind + f' {whex(b)}:\n'
r += s.str1(i + 4) + '\n'
r += ind + ']'
return r
@@ -1131,16 +1142,16 @@ def output_code(self, i, extracted, outerbits, outermask):
if sh > 0:
# Propagate SH down into the local functions.
def str_switch(b, sh=sh):
- return '(insn >> {0}) & 0x{1:x}'.format(sh, b >> sh)
+ return f'(insn >> {sh}) & {b >> sh:#x}'
def str_case(b, sh=sh):
- return '0x{0:x}'.format(b >> sh)
+ return hex(b >> sh)
else:
def str_switch(b):
- return 'insn & 0x{0:08x}'.format(b)
+ return f'insn & {whexC(b)}'
def str_case(b):
- return '0x{0:08x}'.format(b)
+ return whexC(b)
output(ind, 'switch (', str_switch(self.mask), ') {\n')
for b, s in sorted(self.subs):
@@ -1162,8 +1173,7 @@ def __init__(self, m, w):
self.width = w
def str1(self, i):
- ind = str_indent(i)
- return '{0}{1:08x}'.format(ind, self.mask)
+ return str_indent(i) + whex(self.mask)
def __str__(self):
return self.str1(0)
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* RE: [PATCH v3 01/30] decodetree: Introduce whex and whexC helpers
2021-04-30 1:15 ` [PATCH v3 01/30] decodetree: Introduce whex and whexC helpers Richard Henderson
@ 2021-04-30 13:01 ` Luis Fernando Fujita Pires
2021-05-03 22:32 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 63+ messages in thread
From: Luis Fernando Fujita Pires @ 2021-04-30 13:01 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: f4bug, qemu-ppc, lagarcia, Bruno Piazera Larsen,
Matheus Kowalczuk Ferst, david
From: Richard Henderson <richard.henderson@linaro.org>
> Form a hex constant of the appropriate insnwidth.
> Begin using f-strings on changed lines.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> scripts/decodetree.py | 66 +++++++++++++++++++++++++------------------
> 1 file changed, 38 insertions(+), 28 deletions(-)
Reviewed-by: Luis Pires <luis.pires@eldorado.org.br>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 01/30] decodetree: Introduce whex and whexC helpers
2021-04-30 1:15 ` [PATCH v3 01/30] decodetree: Introduce whex and whexC helpers Richard Henderson
2021-04-30 13:01 ` Luis Fernando Fujita Pires
@ 2021-05-03 22:32 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 63+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-03 22:32 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: luis.pires, qemu-ppc, lagarcia, bruno.larsen, matheus.ferst, david
On 4/30/21 3:15 AM, Richard Henderson wrote:
> Form a hex constant of the appropriate insnwidth.
> Begin using f-strings on changed lines.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> scripts/decodetree.py | 66 +++++++++++++++++++++++++------------------
> 1 file changed, 38 insertions(+), 28 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH v3 02/30] decodetree: More use of f-strings
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
2021-04-30 1:15 ` [PATCH v3 01/30] decodetree: Introduce whex and whexC helpers Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 13:01 ` Luis Fernando Fujita Pires
2021-05-03 22:33 ` Philippe Mathieu-Daudé
2021-04-30 1:15 ` [PATCH v3 03/30] decodetree: Add support for 64-bit instructions Richard Henderson
` (28 subsequent siblings)
30 siblings, 2 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
scripts/decodetree.py | 50 ++++++++++++++++++++-----------------------
1 file changed, 23 insertions(+), 27 deletions(-)
diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 0861e5d503..d5da101167 100644
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -59,9 +59,9 @@ def error_with_file(file, lineno, *args):
prefix = ''
if file:
- prefix += '{0}:'.format(file)
+ prefix += f'{file}:'
if lineno:
- prefix += '{0}:'.format(lineno)
+ prefix += f'{lineno}:'
if prefix:
prefix += ' '
print(prefix, end='error: ', file=sys.stderr)
@@ -203,7 +203,7 @@ def str_extract(self):
extr = 'sextract32'
else:
extr = 'extract32'
- return '{0}(insn, {1}, {2})'.format(extr, self.pos, self.len)
+ return f'{extr}(insn, {self.pos}, {self.len})'
def __eq__(self, other):
return self.sign == other.sign and self.mask == other.mask
@@ -227,11 +227,11 @@ def str_extract(self):
ret = '0'
pos = 0
for f in reversed(self.subs):
+ ext = f.str_extract()
if pos == 0:
- ret = f.str_extract()
+ ret = ext
else:
- ret = 'deposit32({0}, {1}, {2}, {3})' \
- .format(ret, pos, 32 - pos, f.str_extract())
+ ret = f'deposit32({ret}, {pos}, {32 - pos}, {ext})'
pos += f.len
return ret
@@ -675,11 +675,11 @@ def parse_field(lineno, name, toks):
subtoks = t.split(':')
sign = False
else:
- error(lineno, 'invalid field token "{0}"'.format(t))
+ error(lineno, f'invalid field token "{t}"')
po = int(subtoks[0])
le = int(subtoks[1])
if po + le > insnwidth:
- error(lineno, 'field {0} too large'.format(t))
+ error(lineno, f'field {t} too large')
f = Field(sign, po, le)
subs.append(f)
width += le
@@ -724,9 +724,9 @@ def parse_arguments(lineno, name, toks):
anyextern = True
continue
if not re.fullmatch(re_C_ident, t):
- error(lineno, 'invalid argument set token "{0}"'.format(t))
+ error(lineno, f'invalid argument set token "{t}"')
if t in flds:
- error(lineno, 'duplicate argument "{0}"'.format(t))
+ error(lineno, f'duplicate argument "{t}"')
flds.append(t)
if name in arguments:
@@ -895,14 +895,14 @@ def parse_generic(lineno, parent_pat, name, toks):
flen = flen[1:]
shift = int(flen, 10)
if shift + width > insnwidth:
- error(lineno, 'field {0} exceeds insnwidth'.format(fname))
+ error(lineno, f'field {fname} exceeds insnwidth')
f = Field(sign, insnwidth - width - shift, shift)
flds = add_field(lineno, flds, fname, f)
fixedbits <<= shift
fixedmask <<= shift
undefmask <<= shift
else:
- error(lineno, 'invalid token "{0}"'.format(t))
+ error(lineno, f'invalid token "{t}"')
width += shift
if variablewidth and width < insnwidth and width % 8 == 0:
@@ -914,7 +914,7 @@ def parse_generic(lineno, parent_pat, name, toks):
# We should have filled in all of the bits of the instruction.
elif not (is_format and width == 0) and width != insnwidth:
- error(lineno, 'definition has {0} bits'.format(width))
+ error(lineno, f'definition has {width} bits')
# Do not check for fields overlapping fields; one valid usage
# is to be able to duplicate fields via import.
@@ -932,8 +932,7 @@ def parse_generic(lineno, parent_pat, name, toks):
if arg:
for f in flds.keys():
if f not in arg.fields:
- error(lineno, 'field {0} not in argument set {1}'
- .format(f, arg.name))
+ error(lineno, f'field {f} not in argument set {arg.name}')
else:
arg = infer_argument_set(flds)
if name in formats:
@@ -960,13 +959,12 @@ def parse_generic(lineno, parent_pat, name, toks):
arg = fmt.base
for f in flds.keys():
if f not in arg.fields:
- error(lineno, 'field {0} not in argument set {1}'
- .format(f, arg.name))
+ error(lineno, f'field {f} not in argument set {arg.name}')
if f in fmt.fields.keys():
- error(lineno, 'field {0} set by format and pattern'.format(f))
+ error(lineno, f'field {f} set by format and pattern')
for f in arg.fields:
if f not in flds.keys() and f not in fmt.fields.keys():
- error(lineno, 'field {0} not initialized'.format(f))
+ error(lineno, f'field {f} not initialized')
pat = Pattern(name, lineno, fmt, fixedbits, fixedmask,
undefmask, fieldmask, flds, width)
parent_pat.pats.append(pat)
@@ -1097,7 +1095,7 @@ def parse_file(f, parent_pat):
elif re.fullmatch(re_pat_ident, name):
parse_generic(start_lineno, parent_pat, name, toks)
else:
- error(lineno, 'invalid token "{0}"'.format(name))
+ error(lineno, f'invalid token "{name}"')
toks = []
if nesting != 0:
@@ -1131,9 +1129,8 @@ def output_code(self, i, extracted, outerbits, outermask):
# If we need to load more bytes to test, do so now.
if extracted < self.width:
- output(ind, 'insn = ', decode_function,
- '_load_bytes(ctx, insn, {0}, {1});\n'
- .format(extracted // 8, self.width // 8));
+ output(ind, f'insn = {decode_function}_load_bytes',
+ f'(ctx, insn, {extracted // 8}, {self.width // 8});\n')
extracted = self.width
# Attempt to aid the compiler in producing compact switch statements.
@@ -1184,9 +1181,8 @@ def output_code(self, i, extracted, outerbits, outermask):
# If we need to load more bytes, do so now.
if extracted < self.width:
- output(ind, 'insn = ', decode_function,
- '_load_bytes(ctx, insn, {0}, {1});\n'
- .format(extracted // 8, self.width // 8));
+ output(ind, f'insn = {decode_function}_load_bytes',
+ f'(ctx, insn, {extracted // 8}, {self.width // 8});\n')
extracted = self.width
output(ind, 'return insn;\n')
# end SizeLeaf
@@ -1220,7 +1216,7 @@ def build_size_tree(pats, width, outerbits, outermask):
for p in pats:
pnames.append(p.name + ':' + p.file + ':' + str(p.lineno))
error_with_file(pats[0].file, pats[0].lineno,
- 'overlapping patterns size {0}:'.format(width), pnames)
+ f'overlapping patterns size {width}:', pnames)
bins = {}
for i in pats:
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* RE: [PATCH v3 02/30] decodetree: More use of f-strings
2021-04-30 1:15 ` [PATCH v3 02/30] decodetree: More use of f-strings Richard Henderson
@ 2021-04-30 13:01 ` Luis Fernando Fujita Pires
2021-05-03 22:33 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 63+ messages in thread
From: Luis Fernando Fujita Pires @ 2021-04-30 13:01 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: f4bug, qemu-ppc, lagarcia, Bruno Piazera Larsen,
Matheus Kowalczuk Ferst, david
From: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> scripts/decodetree.py | 50 ++++++++++++++++++++-----------------------
> 1 file changed, 23 insertions(+), 27 deletions(-)
Reviewed-by: Luis Pires <luis.pires@eldorado.org.br>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 02/30] decodetree: More use of f-strings
2021-04-30 1:15 ` [PATCH v3 02/30] decodetree: More use of f-strings Richard Henderson
2021-04-30 13:01 ` Luis Fernando Fujita Pires
@ 2021-05-03 22:33 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 63+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-03 22:33 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: luis.pires, qemu-ppc, lagarcia, bruno.larsen, matheus.ferst, david
On 4/30/21 3:15 AM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> scripts/decodetree.py | 50 ++++++++++++++++++++-----------------------
> 1 file changed, 23 insertions(+), 27 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH v3 03/30] decodetree: Add support for 64-bit instructions
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
2021-04-30 1:15 ` [PATCH v3 01/30] decodetree: Introduce whex and whexC helpers Richard Henderson
2021-04-30 1:15 ` [PATCH v3 02/30] decodetree: More use of f-strings Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 13:03 ` Luis Fernando Fujita Pires
2021-04-30 1:15 ` [PATCH v3 04/30] decodetree: Extend argument set syntax to allow types Richard Henderson
` (27 subsequent siblings)
30 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
From: Luis Fernando Fujita Pires <luis.pires@eldorado.org.br>
Allow '64' to be specified for the instruction width command line params
and use the appropriate extract and deposit functions in that case.
This will be used to implement the new 64-bit Power ISA 3.1 instructions.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Luis Pires <luis.pires@eldorado.org.br>
Message-Id: <CP2PR80MB3668E123E2EFDB0ACD3A46F1DA759@CP2PR80MB3668.lamprd80.prod.outlook.com>
[rth: Drop the change to the field type; use bitop_width instead of separate
variables for extract/deposit; use "ull" for 64-bit constants.]
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
scripts/decodetree.py | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index d5da101167..f85da45ee3 100644
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -27,6 +27,7 @@
import getopt
insnwidth = 32
+bitop_width = 32
insnmask = 0xffffffff
variablewidth = False
fields = {}
@@ -112,7 +113,9 @@ def whexC(val):
"""Return a hex string for val padded for insnwidth,
and with the proper suffix for a C constant."""
suffix = ''
- if val >= 0x80000000:
+ if val >= 0x100000000:
+ suffix = 'ull'
+ elif val >= 0x80000000:
suffix = 'u'
return whex(val) + suffix
@@ -199,11 +202,9 @@ def __str__(self):
return str(self.pos) + ':' + s + str(self.len)
def str_extract(self):
- if self.sign:
- extr = 'sextract32'
- else:
- extr = 'extract32'
- return f'{extr}(insn, {self.pos}, {self.len})'
+ global bitop_width
+ s = 's' if self.sign else ''
+ return f'{s}extract{bitop_width}(insn, {self.pos}, {self.len})'
def __eq__(self, other):
return self.sign == other.sign and self.mask == other.mask
@@ -224,6 +225,7 @@ def __str__(self):
return str(self.subs)
def str_extract(self):
+ global bitop_width
ret = '0'
pos = 0
for f in reversed(self.subs):
@@ -231,7 +233,7 @@ def str_extract(self):
if pos == 0:
ret = ext
else:
- ret = f'deposit32({ret}, {pos}, {32 - pos}, {ext})'
+ ret = f'deposit{bitop_width}({ret}, {pos}, {bitop_width - pos}, {ext})'
pos += f.len
return ret
@@ -1270,6 +1272,7 @@ def main():
global insntype
global insnmask
global decode_function
+ global bitop_width
global variablewidth
global anyextern
@@ -1299,6 +1302,10 @@ def main():
if insnwidth == 16:
insntype = 'uint16_t'
insnmask = 0xffff
+ elif insnwidth == 64:
+ insntype = 'uint64_t'
+ insnmask = 0xffffffffffffffff
+ bitop_width = 64
elif insnwidth != 32:
error(0, 'cannot handle insns of width', insnwidth)
else:
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* RE: [PATCH v3 03/30] decodetree: Add support for 64-bit instructions
2021-04-30 1:15 ` [PATCH v3 03/30] decodetree: Add support for 64-bit instructions Richard Henderson
@ 2021-04-30 13:03 ` Luis Fernando Fujita Pires
0 siblings, 0 replies; 63+ messages in thread
From: Luis Fernando Fujita Pires @ 2021-04-30 13:03 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: f4bug, qemu-ppc, lagarcia, Bruno Piazera Larsen,
Matheus Kowalczuk Ferst, david
From: Richard Henderson <richard.henderson@linaro.org>
> From: Luis Fernando Fujita Pires <luis.pires@eldorado.org.br>
>
> Allow '64' to be specified for the instruction width command line params and use
> the appropriate extract and deposit functions in that case.
>
> This will be used to implement the new 64-bit Power ISA 3.1 instructions.
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Luis Pires <luis.pires@eldorado.org.br>
> Message-Id:
> <CP2PR80MB3668E123E2EFDB0ACD3A46F1DA759@CP2PR80MB3668.lamprd80
> .prod.outlook.com>
> [rth: Drop the change to the field type; use bitop_width instead of separate
> variables for extract/deposit; use "ull" for 64-bit constants.]
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> scripts/decodetree.py | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
Reviewed-by: Luis Pires <luis.pires@eldorado.org.br>
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH v3 04/30] decodetree: Extend argument set syntax to allow types
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (2 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 03/30] decodetree: Add support for 64-bit instructions Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 13:29 ` Luis Fernando Fujita Pires
2021-04-30 1:15 ` [PATCH v3 05/30] target/ppc: Add cia field to DisasContext Richard Henderson
` (26 subsequent siblings)
30 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Rather than force all structure members to be 'int',
allow the type of the member to be specified.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
docs/devel/decodetree.rst | 11 ++++---
tests/decode/succ_argset_type1.decode | 1 +
scripts/decodetree.py | 45 +++++++++++++++++----------
3 files changed, 36 insertions(+), 21 deletions(-)
create mode 100644 tests/decode/succ_argset_type1.decode
diff --git a/docs/devel/decodetree.rst b/docs/devel/decodetree.rst
index 74f66bf46e..49ea50c2a7 100644
--- a/docs/devel/decodetree.rst
+++ b/docs/devel/decodetree.rst
@@ -40,9 +40,6 @@ and returns an integral value extracted from there.
A field with no ``unnamed_fields`` and no ``!function`` is in error.
-FIXME: the fields of the structure into which this result will be stored
-is restricted to ``int``. Which means that we cannot expand 64-bit items.
-
Field examples:
+---------------------------+---------------------------------------------+
@@ -66,9 +63,14 @@ Argument Sets
Syntax::
args_def := '&' identifier ( args_elt )+ ( !extern )?
- args_elt := identifier
+ args_elt := identifier (':' identifier)?
Each *args_elt* defines an argument within the argument set.
+If the form of the *args_elt* contains a colon, the first
+identifier is the argument name and the second identifier is
+the argument type. If the colon is missing, the argument
+type will be ``int``.
+
Each argument set will be rendered as a C structure "arg_$name"
with each of the fields being one of the member arguments.
@@ -86,6 +88,7 @@ Argument set examples::
®3 ra rb rc
&loadstore reg base offset
+ &longldst reg base offset:int64_t
Formats
diff --git a/tests/decode/succ_argset_type1.decode b/tests/decode/succ_argset_type1.decode
new file mode 100644
index 0000000000..ed946b420d
--- /dev/null
+++ b/tests/decode/succ_argset_type1.decode
@@ -0,0 +1 @@
+&asdf b:bool c:uint64_t a
diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index f85da45ee3..a03dc6b5e3 100644
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -165,11 +165,15 @@ def is_contiguous(bits):
return -1
-def eq_fields_for_args(flds_a, flds_b):
- if len(flds_a) != len(flds_b):
+def eq_fields_for_args(flds_a, arg):
+ if len(flds_a) != len(arg.fields):
return False
+ # Only allow inference on default types
+ for t in arg.types:
+ if t != 'int':
+ return False
for k, a in flds_a.items():
- if k not in flds_b:
+ if k not in arg.fields:
return False
return True
@@ -313,10 +317,11 @@ def __ne__(self, other):
class Arguments:
"""Class representing the extracted fields of a format"""
- def __init__(self, nm, flds, extern):
+ def __init__(self, nm, flds, types, extern):
self.name = nm
self.extern = extern
- self.fields = sorted(flds)
+ self.fields = flds
+ self.types = types
def __str__(self):
return self.name + ' ' + str(self.fields)
@@ -327,8 +332,8 @@ def struct_name(self):
def output_def(self):
if not self.extern:
output('typedef struct {\n')
- for n in self.fields:
- output(' int ', n, ';\n')
+ for (n, t) in zip(self.fields, self.types):
+ output(f' {t} {n};\n')
output('} ', self.struct_name(), ';\n\n')
# end Arguments
@@ -719,21 +724,27 @@ def parse_arguments(lineno, name, toks):
global anyextern
flds = []
+ types = []
extern = False
- for t in toks:
- if re.fullmatch('!extern', t):
+ for n in toks:
+ if re.fullmatch('!extern', n):
extern = True
anyextern = True
continue
- if not re.fullmatch(re_C_ident, t):
- error(lineno, f'invalid argument set token "{t}"')
- if t in flds:
- error(lineno, f'duplicate argument "{t}"')
- flds.append(t)
+ if re.fullmatch(re_C_ident + ':' + re_C_ident, n):
+ (n, t) = n.split(':')
+ elif re.fullmatch(re_C_ident, n):
+ t = 'int'
+ else:
+ error(lineno, f'invalid argument set token "{n}"')
+ if n in flds:
+ error(lineno, f'duplicate argument "{n}"')
+ flds.append(n)
+ types.append(t)
if name in arguments:
error(lineno, 'duplicate argument set', name)
- arguments[name] = Arguments(name, flds, extern)
+ arguments[name] = Arguments(name, flds, types, extern)
# end parse_arguments
@@ -760,11 +771,11 @@ def infer_argument_set(flds):
global decode_function
for arg in arguments.values():
- if eq_fields_for_args(flds, arg.fields):
+ if eq_fields_for_args(flds, arg):
return arg
name = decode_function + str(len(arguments))
- arg = Arguments(name, flds.keys(), False)
+ arg = Arguments(name, flds.keys(), ['int'] * len(flds), False)
arguments[name] = arg
return arg
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* RE: [PATCH v3 04/30] decodetree: Extend argument set syntax to allow types
2021-04-30 1:15 ` [PATCH v3 04/30] decodetree: Extend argument set syntax to allow types Richard Henderson
@ 2021-04-30 13:29 ` Luis Fernando Fujita Pires
0 siblings, 0 replies; 63+ messages in thread
From: Luis Fernando Fujita Pires @ 2021-04-30 13:29 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: f4bug, qemu-ppc, lagarcia, Bruno Piazera Larsen,
Matheus Kowalczuk Ferst, david
From: Richard Henderson <richard.henderson@linaro.org>
> Rather than force all structure members to be 'int', allow the type of the
> member to be specified.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> docs/devel/decodetree.rst | 11 ++++---
> tests/decode/succ_argset_type1.decode | 1 +
> scripts/decodetree.py | 45 +++++++++++++++++----------
> 3 files changed, 36 insertions(+), 21 deletions(-) create mode 100644
> tests/decode/succ_argset_type1.decode
Reviewed-by: Luis Pires <luis.pires@eldorado.org.br>
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH v3 05/30] target/ppc: Add cia field to DisasContext
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (3 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 04/30] decodetree: Extend argument set syntax to allow types Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 20:08 ` Bruno Piazera Larsen
2021-04-30 20:35 ` Luis Fernando Fujita Pires
2021-04-30 1:15 ` [PATCH v3 06/30] target/ppc: Split out decode_legacy Richard Henderson
` (25 subsequent siblings)
30 siblings, 2 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/translate.c | 34 ++++++++++++++++++----------------
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 0984ce637b..ee25badba2 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -154,6 +154,7 @@ void ppc_translate_init(void)
/* internal defines */
struct DisasContext {
DisasContextBase base;
+ target_ulong cia; /* current instruction address */
uint32_t opcode;
uint32_t exception;
/* Routine used to access memory */
@@ -254,7 +255,7 @@ static void gen_exception_err(DisasContext *ctx, uint32_t excp, uint32_t error)
* faulting instruction
*/
if (ctx->exception == POWERPC_EXCP_NONE) {
- gen_update_nip(ctx, ctx->base.pc_next - 4);
+ gen_update_nip(ctx, ctx->cia);
}
t0 = tcg_const_i32(excp);
t1 = tcg_const_i32(error);
@@ -273,7 +274,7 @@ static void gen_exception(DisasContext *ctx, uint32_t excp)
* faulting instruction
*/
if (ctx->exception == POWERPC_EXCP_NONE) {
- gen_update_nip(ctx, ctx->base.pc_next - 4);
+ gen_update_nip(ctx, ctx->cia);
}
t0 = tcg_const_i32(excp);
gen_helper_raise_exception(cpu_env, t0);
@@ -3113,7 +3114,7 @@ static void gen_eieio(DisasContext *ctx)
*/
if (!(ctx->insns_flags2 & PPC2_ISA300)) {
qemu_log_mask(LOG_GUEST_ERROR, "invalid eieio using bit 6 at @"
- TARGET_FMT_lx "\n", ctx->base.pc_next - 4);
+ TARGET_FMT_lx "\n", ctx->cia);
} else {
bar = TCG_MO_ST_LD;
}
@@ -3782,14 +3783,14 @@ static void gen_b(DisasContext *ctx)
li = LI(ctx->opcode);
li = (li ^ 0x02000000) - 0x02000000;
if (likely(AA(ctx->opcode) == 0)) {
- target = ctx->base.pc_next + li - 4;
+ target = ctx->cia + li;
} else {
target = li;
}
if (LK(ctx->opcode)) {
gen_setlr(ctx, ctx->base.pc_next);
}
- gen_update_cfar(ctx, ctx->base.pc_next - 4);
+ gen_update_cfar(ctx, ctx->cia);
gen_goto_tb(ctx, 0, target);
}
@@ -3888,11 +3889,11 @@ static void gen_bcond(DisasContext *ctx, int type)
}
tcg_temp_free_i32(temp);
}
- gen_update_cfar(ctx, ctx->base.pc_next - 4);
+ gen_update_cfar(ctx, ctx->cia);
if (type == BCOND_IM) {
target_ulong li = (target_long)((int16_t)(BD(ctx->opcode)));
if (likely(AA(ctx->opcode) == 0)) {
- gen_goto_tb(ctx, 0, ctx->base.pc_next + li - 4);
+ gen_goto_tb(ctx, 0, ctx->cia + li);
} else {
gen_goto_tb(ctx, 0, li);
}
@@ -4008,7 +4009,7 @@ static void gen_rfi(DisasContext *ctx)
if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
gen_io_start();
}
- gen_update_cfar(ctx, ctx->base.pc_next - 4);
+ gen_update_cfar(ctx, ctx->cia);
gen_helper_rfi(cpu_env);
gen_sync_exception(ctx);
#endif
@@ -4025,7 +4026,7 @@ static void gen_rfid(DisasContext *ctx)
if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
gen_io_start();
}
- gen_update_cfar(ctx, ctx->base.pc_next - 4);
+ gen_update_cfar(ctx, ctx->cia);
gen_helper_rfid(cpu_env);
gen_sync_exception(ctx);
#endif
@@ -4042,7 +4043,7 @@ static void gen_rfscv(DisasContext *ctx)
if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
gen_io_start();
}
- gen_update_cfar(ctx, ctx->base.pc_next - 4);
+ gen_update_cfar(ctx, ctx->cia);
gen_helper_rfscv(cpu_env);
gen_sync_exception(ctx);
#endif
@@ -4338,7 +4339,7 @@ static inline void gen_op_mfspr(DisasContext *ctx)
if (sprn != SPR_PVR) {
qemu_log_mask(LOG_GUEST_ERROR, "Trying to read privileged spr "
"%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn,
- ctx->base.pc_next - 4);
+ ctx->cia);
}
gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
}
@@ -4352,7 +4353,7 @@ static inline void gen_op_mfspr(DisasContext *ctx)
/* Not defined */
qemu_log_mask(LOG_GUEST_ERROR,
"Trying to read invalid spr %d (0x%03x) at "
- TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4);
+ TARGET_FMT_lx "\n", sprn, sprn, ctx->cia);
/*
* The behaviour depends on MSR:PR and SPR# bit 0x10, it can
@@ -4516,7 +4517,7 @@ static void gen_mtspr(DisasContext *ctx)
/* Privilege exception */
qemu_log_mask(LOG_GUEST_ERROR, "Trying to write privileged spr "
"%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn,
- ctx->base.pc_next - 4);
+ ctx->cia);
gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
}
} else {
@@ -4530,7 +4531,7 @@ static void gen_mtspr(DisasContext *ctx)
/* Not defined */
qemu_log_mask(LOG_GUEST_ERROR,
"Trying to write invalid spr %d (0x%03x) at "
- TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4);
+ TARGET_FMT_lx "\n", sprn, sprn, ctx->cia);
/*
@@ -8002,6 +8003,7 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
LOG_DISAS("nip=" TARGET_FMT_lx " super=%d ir=%d\n",
ctx->base.pc_next, ctx->mem_idx, (int)msr_ir);
+ ctx->cia = ctx->base.pc_next;
ctx->opcode = translator_ldl_swap(env, ctx->base.pc_next,
need_byteswap(ctx));
@@ -8031,7 +8033,7 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
TARGET_FMT_lx " %d\n",
opc1(ctx->opcode), opc2(ctx->opcode),
opc3(ctx->opcode), opc4(ctx->opcode),
- ctx->opcode, ctx->base.pc_next - 4, (int)msr_ir);
+ ctx->opcode, ctx->cia, (int)msr_ir);
} else {
uint32_t inval;
@@ -8048,7 +8050,7 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
TARGET_FMT_lx "\n", ctx->opcode & inval,
opc1(ctx->opcode), opc2(ctx->opcode),
opc3(ctx->opcode), opc4(ctx->opcode),
- ctx->opcode, ctx->base.pc_next - 4);
+ ctx->opcode, ctx->cia);
gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
ctx->base.is_jmp = DISAS_NORETURN;
return;
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH v3 05/30] target/ppc: Add cia field to DisasContext
2021-04-30 1:15 ` [PATCH v3 05/30] target/ppc: Add cia field to DisasContext Richard Henderson
@ 2021-04-30 20:08 ` Bruno Piazera Larsen
2021-04-30 20:35 ` Luis Fernando Fujita Pires
1 sibling, 0 replies; 63+ messages in thread
From: Bruno Piazera Larsen @ 2021-04-30 20:08 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, matheus.ferst, david
[-- Attachment #1: Type: text/plain, Size: 611 bytes --]
On 29/04/2021 22:15, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/ppc/translate.c | 34 ++++++++++++++++++----------------
> 1 file changed, 18 insertions(+), 16 deletions(-)
Reviewed-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
--
Bruno Piazera Larsen
Instituto de Pesquisas ELDORADO
<https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station>
Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
[-- Attachment #2: Type: text/html, Size: 1557 bytes --]
^ permalink raw reply [flat|nested] 63+ messages in thread
* RE: [PATCH v3 05/30] target/ppc: Add cia field to DisasContext
2021-04-30 1:15 ` [PATCH v3 05/30] target/ppc: Add cia field to DisasContext Richard Henderson
2021-04-30 20:08 ` Bruno Piazera Larsen
@ 2021-04-30 20:35 ` Luis Fernando Fujita Pires
1 sibling, 0 replies; 63+ messages in thread
From: Luis Fernando Fujita Pires @ 2021-04-30 20:35 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: f4bug, qemu-ppc, lagarcia, Bruno Piazera Larsen,
Matheus Kowalczuk Ferst, david
> From: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/ppc/translate.c | 34 ++++++++++++++++++----------------
> 1 file changed, 18 insertions(+), 16 deletions(-)
Reviewed-by: Luis Pires <luis.pires@eldorado.org.br>
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH v3 06/30] target/ppc: Split out decode_legacy
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (4 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 05/30] target/ppc: Add cia field to DisasContext Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 20:36 ` Luis Fernando Fujita Pires
2021-04-30 1:15 ` [PATCH v3 07/30] target/ppc: Move DISAS_NORETURN setting into gen_exception* Richard Henderson
` (24 subsequent siblings)
30 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/translate.c | 115 +++++++++++++++++++++++------------------
1 file changed, 64 insertions(+), 51 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index ee25badba2..ebe5afe7ae 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7876,6 +7876,62 @@ void ppc_cpu_dump_statistics(CPUState *cs, int flags)
#endif
}
+static bool decode_legacy(PowerPCCPU *cpu, DisasContext *ctx, uint32_t insn)
+{
+ opc_handler_t **table, *handler;
+ uint32_t inval;
+
+ ctx->opcode = insn;
+
+ LOG_DISAS("translate opcode %08x (%02x %02x %02x %02x) (%s)\n",
+ insn, opc1(insn), opc2(insn), opc3(insn), opc4(insn),
+ ctx->le_mode ? "little" : "big");
+
+ table = cpu->opcodes;
+ handler = table[opc1(insn)];
+ if (is_indirect_opcode(handler)) {
+ table = ind_table(handler);
+ handler = table[opc2(insn)];
+ if (is_indirect_opcode(handler)) {
+ table = ind_table(handler);
+ handler = table[opc3(insn)];
+ if (is_indirect_opcode(handler)) {
+ table = ind_table(handler);
+ handler = table[opc4(insn)];
+ }
+ }
+ }
+
+ /* Is opcode *REALLY* valid ? */
+ if (unlikely(handler->handler == &gen_invalid)) {
+ qemu_log_mask(LOG_GUEST_ERROR, "invalid/unsupported opcode: "
+ "%02x - %02x - %02x - %02x (%08x) "
+ TARGET_FMT_lx "\n",
+ opc1(insn), opc2(insn), opc3(insn), opc4(insn),
+ insn, ctx->cia);
+ return false;
+ }
+
+ if (unlikely(handler->type & (PPC_SPE | PPC_SPE_SINGLE | PPC_SPE_DOUBLE)
+ && Rc(insn))) {
+ inval = handler->inval2;
+ } else {
+ inval = handler->inval1;
+ }
+
+ if (unlikely((insn & inval) != 0)) {
+ qemu_log_mask(LOG_GUEST_ERROR, "invalid bits: %08x for opcode: "
+ "%02x - %02x - %02x - %02x (%08x) "
+ TARGET_FMT_lx "\n", insn & inval,
+ opc1(insn), opc2(insn), opc3(insn), opc4(insn),
+ insn, ctx->cia);
+ return false;
+ }
+
+ handler->handler(ctx);
+ return true;
+}
+
static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
{
DisasContext *ctx = container_of(dcbase, DisasContext, base);
@@ -7997,66 +8053,23 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
DisasContext *ctx = container_of(dcbase, DisasContext, base);
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = cs->env_ptr;
- opc_handler_t **table, *handler;
+ uint32_t insn;
+ bool ok;
LOG_DISAS("----------------\n");
LOG_DISAS("nip=" TARGET_FMT_lx " super=%d ir=%d\n",
ctx->base.pc_next, ctx->mem_idx, (int)msr_ir);
ctx->cia = ctx->base.pc_next;
- ctx->opcode = translator_ldl_swap(env, ctx->base.pc_next,
- need_byteswap(ctx));
-
- LOG_DISAS("translate opcode %08x (%02x %02x %02x %02x) (%s)\n",
- ctx->opcode, opc1(ctx->opcode), opc2(ctx->opcode),
- opc3(ctx->opcode), opc4(ctx->opcode),
- ctx->le_mode ? "little" : "big");
+ insn = translator_ldl_swap(env, ctx->base.pc_next, need_byteswap(ctx));
ctx->base.pc_next += 4;
- table = cpu->opcodes;
- handler = table[opc1(ctx->opcode)];
- if (is_indirect_opcode(handler)) {
- table = ind_table(handler);
- handler = table[opc2(ctx->opcode)];
- if (is_indirect_opcode(handler)) {
- table = ind_table(handler);
- handler = table[opc3(ctx->opcode)];
- if (is_indirect_opcode(handler)) {
- table = ind_table(handler);
- handler = table[opc4(ctx->opcode)];
- }
- }
- }
- /* Is opcode *REALLY* valid ? */
- if (unlikely(handler->handler == &gen_invalid)) {
- qemu_log_mask(LOG_GUEST_ERROR, "invalid/unsupported opcode: "
- "%02x - %02x - %02x - %02x (%08x) "
- TARGET_FMT_lx " %d\n",
- opc1(ctx->opcode), opc2(ctx->opcode),
- opc3(ctx->opcode), opc4(ctx->opcode),
- ctx->opcode, ctx->cia, (int)msr_ir);
- } else {
- uint32_t inval;
- if (unlikely(handler->type & (PPC_SPE | PPC_SPE_SINGLE | PPC_SPE_DOUBLE)
- && Rc(ctx->opcode))) {
- inval = handler->inval2;
- } else {
- inval = handler->inval1;
- }
-
- if (unlikely((ctx->opcode & inval) != 0)) {
- qemu_log_mask(LOG_GUEST_ERROR, "invalid bits: %08x for opcode: "
- "%02x - %02x - %02x - %02x (%08x) "
- TARGET_FMT_lx "\n", ctx->opcode & inval,
- opc1(ctx->opcode), opc2(ctx->opcode),
- opc3(ctx->opcode), opc4(ctx->opcode),
- ctx->opcode, ctx->cia);
- gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
- ctx->base.is_jmp = DISAS_NORETURN;
- return;
- }
+ ok = decode_legacy(cpu, ctx, insn);
+ if (!ok) {
+ gen_invalid(ctx);
+ ctx->base.is_jmp = DISAS_NORETURN;
}
- (*(handler->handler))(ctx);
+
#if defined(DO_PPC_STATISTICS)
handler->count++;
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH v3 07/30] target/ppc: Move DISAS_NORETURN setting into gen_exception*
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (5 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 06/30] target/ppc: Split out decode_legacy Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-05-03 12:58 ` Luis Fernando Fujita Pires
2021-04-30 1:15 ` [PATCH v3 08/30] target/ppc: Remove special case for POWERPC_SYSCALL Richard Henderson
` (23 subsequent siblings)
30 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
There are other valid settings for is_jmp besides
DISAS_NEXT and DISAS_NORETURN, so eliminating that
dichotomy from ppc_tr_translate_insn is helpful.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v3: Retain an exit from translator loop for ctx->exception.
Do not emit code for single-step or ppc_tr_tb_stop for NORETURN.
---
target/ppc/translate.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index ebe5afe7ae..3607cc12f3 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -262,7 +262,8 @@ static void gen_exception_err(DisasContext *ctx, uint32_t excp, uint32_t error)
gen_helper_raise_exception_err(cpu_env, t0, t1);
tcg_temp_free_i32(t0);
tcg_temp_free_i32(t1);
- ctx->exception = (excp);
+ ctx->exception = excp;
+ ctx->base.is_jmp = DISAS_NORETURN;
}
static void gen_exception(DisasContext *ctx, uint32_t excp)
@@ -279,7 +280,8 @@ static void gen_exception(DisasContext *ctx, uint32_t excp)
t0 = tcg_const_i32(excp);
gen_helper_raise_exception(cpu_env, t0);
tcg_temp_free_i32(t0);
- ctx->exception = (excp);
+ ctx->exception = excp;
+ ctx->base.is_jmp = DISAS_NORETURN;
}
static void gen_exception_nip(DisasContext *ctx, uint32_t excp,
@@ -291,7 +293,8 @@ static void gen_exception_nip(DisasContext *ctx, uint32_t excp,
t0 = tcg_const_i32(excp);
gen_helper_raise_exception(cpu_env, t0);
tcg_temp_free_i32(t0);
- ctx->exception = (excp);
+ ctx->exception = excp;
+ ctx->base.is_jmp = DISAS_NORETURN;
}
/*
@@ -337,6 +340,7 @@ static void gen_debug_exception(DisasContext *ctx)
t0 = tcg_const_i32(EXCP_DEBUG);
gen_helper_raise_exception(cpu_env, t0);
tcg_temp_free_i32(t0);
+ ctx->base.is_jmp = DISAS_NORETURN;
}
static inline void gen_inval_exception(DisasContext *ctx, uint32_t error)
@@ -8037,7 +8041,6 @@ static bool ppc_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
DisasContext *ctx = container_of(dcbase, DisasContext, base);
gen_debug_exception(ctx);
- dcbase->is_jmp = DISAS_NORETURN;
/*
* The address covered by the breakpoint must be included in
* [tb->pc, tb->pc + tb->size) in order to for it to be properly
@@ -8067,18 +8070,19 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
ok = decode_legacy(cpu, ctx, insn);
if (!ok) {
gen_invalid(ctx);
- ctx->base.is_jmp = DISAS_NORETURN;
}
#if defined(DO_PPC_STATISTICS)
handler->count++;
#endif
+
/* Check trace mode exceptions */
if (unlikely(ctx->singlestep_enabled & CPU_SINGLE_STEP &&
(ctx->base.pc_next <= 0x100 || ctx->base.pc_next > 0xF00) &&
ctx->exception != POWERPC_SYSCALL &&
ctx->exception != POWERPC_EXCP_TRAP &&
- ctx->exception != POWERPC_EXCP_BRANCH)) {
+ ctx->exception != POWERPC_EXCP_BRANCH &&
+ ctx->base.is_jmp != DISAS_NORETURN)) {
uint32_t excp = gen_prep_dbgex(ctx);
gen_exception_nip(ctx, excp, ctx->base.pc_next);
}
@@ -8089,14 +8093,20 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
opc3(ctx->opcode), opc4(ctx->opcode), ctx->opcode);
}
- ctx->base.is_jmp = ctx->exception == POWERPC_EXCP_NONE ?
- DISAS_NEXT : DISAS_NORETURN;
+ if (ctx->base.is_jmp == DISAS_NEXT
+ && ctx->exception != POWERPC_EXCP_NONE) {
+ ctx->base.is_jmp = DISAS_TOO_MANY;
+ }
}
static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
{
DisasContext *ctx = container_of(dcbase, DisasContext, base);
+ if (ctx->base.is_jmp == DISAS_NORETURN) {
+ return;
+ }
+
if (ctx->exception == POWERPC_EXCP_NONE) {
gen_goto_tb(ctx, 0, ctx->base.pc_next);
} else if (ctx->exception != POWERPC_EXCP_BRANCH) {
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* RE: [PATCH v3 07/30] target/ppc: Move DISAS_NORETURN setting into gen_exception*
2021-04-30 1:15 ` [PATCH v3 07/30] target/ppc: Move DISAS_NORETURN setting into gen_exception* Richard Henderson
@ 2021-05-03 12:58 ` Luis Fernando Fujita Pires
0 siblings, 0 replies; 63+ messages in thread
From: Luis Fernando Fujita Pires @ 2021-05-03 12:58 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: f4bug, qemu-ppc, lagarcia, Bruno Piazera Larsen,
Matheus Kowalczuk Ferst, david
From: Richard Henderson <richard.henderson@linaro.org>
> There are other valid settings for is_jmp besides DISAS_NEXT and
> DISAS_NORETURN, so eliminating that dichotomy from ppc_tr_translate_insn is
> helpful.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v3: Retain an exit from translator loop for ctx->exception.
> Do not emit code for single-step or ppc_tr_tb_stop for NORETURN.
> ---
> target/ppc/translate.c | 26 ++++++++++++++++++--------
> 1 file changed, 18 insertions(+), 8 deletions(-)
Reviewed-by: Luis Pires <luis.pires@eldorado.org.br>
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH v3 08/30] target/ppc: Remove special case for POWERPC_SYSCALL
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (6 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 07/30] target/ppc: Move DISAS_NORETURN setting into gen_exception* Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-05-03 12:59 ` Luis Fernando Fujita Pires
2021-04-30 1:15 ` [PATCH v3 09/30] target/ppc: Remove special case for POWERPC_EXCP_TRAP Richard Henderson
` (22 subsequent siblings)
30 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Since POWERPC_SYSCALL is raised by gen_exception_err,
we will have also set DISAS_NORETURN.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/translate.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 3607cc12f3..b26b6964a7 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -8079,7 +8079,6 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
/* Check trace mode exceptions */
if (unlikely(ctx->singlestep_enabled & CPU_SINGLE_STEP &&
(ctx->base.pc_next <= 0x100 || ctx->base.pc_next > 0xF00) &&
- ctx->exception != POWERPC_SYSCALL &&
ctx->exception != POWERPC_EXCP_TRAP &&
ctx->exception != POWERPC_EXCP_BRANCH &&
ctx->base.is_jmp != DISAS_NORETURN)) {
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH v3 09/30] target/ppc: Remove special case for POWERPC_EXCP_TRAP
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (7 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 08/30] target/ppc: Remove special case for POWERPC_SYSCALL Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-05-03 13:00 ` Luis Fernando Fujita Pires
2021-04-30 1:15 ` [PATCH v3 10/30] target/ppc: Simplify gen_debug_exception Richard Henderson
` (21 subsequent siblings)
30 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Since POWERPC_EXCP_TRAP is raised by gen_exception_err,
we will have also set DISAS_NORETURN.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/translate.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index b26b6964a7..5efa4d6566 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -8079,7 +8079,6 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
/* Check trace mode exceptions */
if (unlikely(ctx->singlestep_enabled & CPU_SINGLE_STEP &&
(ctx->base.pc_next <= 0x100 || ctx->base.pc_next > 0xF00) &&
- ctx->exception != POWERPC_EXCP_TRAP &&
ctx->exception != POWERPC_EXCP_BRANCH &&
ctx->base.is_jmp != DISAS_NORETURN)) {
uint32_t excp = gen_prep_dbgex(ctx);
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH v3 10/30] target/ppc: Simplify gen_debug_exception
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (8 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 09/30] target/ppc: Remove special case for POWERPC_EXCP_TRAP Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 1:15 ` [PATCH v3 11/30] target/ppc: Introduce DISAS_{EXIT,CHAIN}{,_UPDATE} Richard Henderson
` (20 subsequent siblings)
30 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Two of the call sites that use gen_debug_exception have already
updated NIP. Only ppc_tr_breakpoint_check requires the update.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/translate.c | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 5efa4d6566..b58e2ac8dc 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -327,19 +327,7 @@ static uint32_t gen_prep_dbgex(DisasContext *ctx)
static void gen_debug_exception(DisasContext *ctx)
{
- TCGv_i32 t0;
-
- /*
- * These are all synchronous exceptions, we set the PC back to the
- * faulting instruction
- */
- if ((ctx->exception != POWERPC_EXCP_BRANCH) &&
- (ctx->exception != POWERPC_EXCP_SYNC)) {
- gen_update_nip(ctx, ctx->base.pc_next);
- }
- t0 = tcg_const_i32(EXCP_DEBUG);
- gen_helper_raise_exception(cpu_env, t0);
- tcg_temp_free_i32(t0);
+ gen_helper_raise_exception(cpu_env, tcg_constant_i32(EXCP_DEBUG));
ctx->base.is_jmp = DISAS_NORETURN;
}
@@ -8040,6 +8028,7 @@ static bool ppc_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
{
DisasContext *ctx = container_of(dcbase, DisasContext, base);
+ gen_update_nip(ctx, ctx->base.pc_next);
gen_debug_exception(ctx);
/*
* The address covered by the breakpoint must be included in
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH v3 11/30] target/ppc: Introduce DISAS_{EXIT,CHAIN}{,_UPDATE}
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (9 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 10/30] target/ppc: Simplify gen_debug_exception Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 1:15 ` [PATCH v3 12/30] target/ppc: Replace POWERPC_EXCP_SYNC with DISAS_EXIT Richard Henderson
` (19 subsequent siblings)
30 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Rewrite ppc_tr_tb_stop to handle these new codes.
Convert ctx->exception into these new codes at the end of
ppc_tr_translate_insn, prior to pushing the change back
throughout translate.c.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/translate.c | 75 ++++++++++++++++++++++++++++++++++++------
1 file changed, 65 insertions(+), 10 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index b58e2ac8dc..7dbdf3d047 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -183,6 +183,11 @@ struct DisasContext {
uint64_t insns_flags2;
};
+#define DISAS_EXIT DISAS_TARGET_0 /* exit to main loop, pc updated */
+#define DISAS_EXIT_UPDATE DISAS_TARGET_1 /* exit to main loop, pc stale */
+#define DISAS_CHAIN DISAS_TARGET_2 /* lookup next tb, pc updated */
+#define DISAS_CHAIN_UPDATE DISAS_TARGET_3 /* lookup next tb, pc stale */
+
/* Return true iff byteswap is needed in a scalar memop */
static inline bool need_byteswap(const DisasContext *ctx)
{
@@ -8080,28 +8085,78 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
opc3(ctx->opcode), opc4(ctx->opcode), ctx->opcode);
}
- if (ctx->base.is_jmp == DISAS_NEXT
- && ctx->exception != POWERPC_EXCP_NONE) {
- ctx->base.is_jmp = DISAS_TOO_MANY;
+ if (ctx->base.is_jmp == DISAS_NEXT) {
+ switch (ctx->exception) {
+ case POWERPC_EXCP_NONE:
+ break;
+ case POWERPC_EXCP_BRANCH:
+ ctx->base.is_jmp = DISAS_NORETURN;
+ break;
+ case POWERPC_EXCP_SYNC:
+ case POWERPC_EXCP_STOP:
+ ctx->base.is_jmp = DISAS_EXIT;
+ break;
+ default:
+ /* Every other ctx->exception should have set NORETURN. */
+ g_assert_not_reached();
+ }
}
}
static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
{
DisasContext *ctx = container_of(dcbase, DisasContext, base);
+ DisasJumpType is_jmp = ctx->base.is_jmp;
+ target_ulong nip = ctx->base.pc_next;
- if (ctx->base.is_jmp == DISAS_NORETURN) {
+ if (is_jmp == DISAS_NORETURN) {
+ /* We have already exited the TB. */
return;
}
- if (ctx->exception == POWERPC_EXCP_NONE) {
- gen_goto_tb(ctx, 0, ctx->base.pc_next);
- } else if (ctx->exception != POWERPC_EXCP_BRANCH) {
- if (unlikely(ctx->base.singlestep_enabled)) {
- gen_debug_exception(ctx);
+ /* Honor single stepping. */
+ if (unlikely(ctx->base.singlestep_enabled)) {
+ switch (is_jmp) {
+ case DISAS_TOO_MANY:
+ case DISAS_EXIT_UPDATE:
+ case DISAS_CHAIN_UPDATE:
+ gen_update_nip(ctx, nip);
+ break;
+ case DISAS_EXIT:
+ case DISAS_CHAIN:
+ break;
+ default:
+ g_assert_not_reached();
}
- /* Generate the return instruction */
+ gen_debug_exception(ctx);
+ return;
+ }
+
+ switch (is_jmp) {
+ case DISAS_TOO_MANY:
+ if (use_goto_tb(ctx, nip)) {
+ tcg_gen_goto_tb(0);
+ gen_update_nip(ctx, nip);
+ tcg_gen_exit_tb(ctx->base.tb, 0);
+ break;
+ }
+ /* fall through */
+ case DISAS_CHAIN_UPDATE:
+ gen_update_nip(ctx, nip);
+ /* fall through */
+ case DISAS_CHAIN:
+ tcg_gen_lookup_and_goto_ptr();
+ break;
+
+ case DISAS_EXIT_UPDATE:
+ gen_update_nip(ctx, nip);
+ /* fall through */
+ case DISAS_EXIT:
tcg_gen_exit_tb(NULL, 0);
+ break;
+
+ default:
+ g_assert_not_reached();
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH v3 12/30] target/ppc: Replace POWERPC_EXCP_SYNC with DISAS_EXIT
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (10 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 11/30] target/ppc: Introduce DISAS_{EXIT,CHAIN}{,_UPDATE} Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 1:15 ` [PATCH v3 13/30] target/ppc: Remove unnecessary gen_io_end calls Richard Henderson
` (18 subsequent siblings)
30 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Remove the synthetic "exception" after no more uses.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/cpu.h | 1 -
target/ppc/translate.c | 27 +++++++++------------------
2 files changed, 9 insertions(+), 19 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index e73416da68..cf10117065 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -135,7 +135,6 @@ enum {
POWERPC_EXCP_STOP = 0x200, /* stop translation */
POWERPC_EXCP_BRANCH = 0x201, /* branch instruction */
/* QEMU exceptions: special cases we want to stop translation */
- POWERPC_EXCP_SYNC = 0x202, /* context synchronizing instruction */
POWERPC_EXCP_SYSCALL_USER = 0x203, /* System call in user mode only */
};
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 7dbdf3d047..d22d6e5b85 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -360,14 +360,6 @@ static inline void gen_stop_exception(DisasContext *ctx)
ctx->exception = POWERPC_EXCP_STOP;
}
-#ifndef CONFIG_USER_ONLY
-/* No need to update nip here, as execution flow will change */
-static inline void gen_sync_exception(DisasContext *ctx)
-{
- ctx->exception = POWERPC_EXCP_SYNC;
-}
-#endif
-
#define GEN_HANDLER(name, opc1, opc2, opc3, inval, type) \
GEN_OPCODE(name, opc1, opc2, opc3, inval, type, PPC_NONE)
@@ -4008,7 +4000,7 @@ static void gen_rfi(DisasContext *ctx)
}
gen_update_cfar(ctx, ctx->cia);
gen_helper_rfi(cpu_env);
- gen_sync_exception(ctx);
+ ctx->base.is_jmp = DISAS_EXIT;
#endif
}
@@ -4025,7 +4017,7 @@ static void gen_rfid(DisasContext *ctx)
}
gen_update_cfar(ctx, ctx->cia);
gen_helper_rfid(cpu_env);
- gen_sync_exception(ctx);
+ ctx->base.is_jmp = DISAS_EXIT;
#endif
}
@@ -4042,7 +4034,7 @@ static void gen_rfscv(DisasContext *ctx)
}
gen_update_cfar(ctx, ctx->cia);
gen_helper_rfscv(cpu_env);
- gen_sync_exception(ctx);
+ ctx->base.is_jmp = DISAS_EXIT;
#endif
}
#endif
@@ -4055,7 +4047,7 @@ static void gen_hrfid(DisasContext *ctx)
/* Restore CPU state */
CHK_HV;
gen_helper_hrfid(cpu_env);
- gen_sync_exception(ctx);
+ ctx->base.is_jmp = DISAS_EXIT;
#endif
}
#endif
@@ -5941,7 +5933,7 @@ static void gen_rfsvc(DisasContext *ctx)
CHK_SV;
gen_helper_rfsvc(cpu_env);
- gen_sync_exception(ctx);
+ ctx->base.is_jmp = DISAS_EXIT;
#endif /* defined(CONFIG_USER_ONLY) */
}
@@ -6321,7 +6313,7 @@ static void gen_rfci_40x(DisasContext *ctx)
CHK_SV;
/* Restore CPU state */
gen_helper_40x_rfci(cpu_env);
- gen_sync_exception(ctx);
+ ctx->base.is_jmp = DISAS_EXIT;
#endif /* defined(CONFIG_USER_ONLY) */
}
@@ -6333,7 +6325,7 @@ static void gen_rfci(DisasContext *ctx)
CHK_SV;
/* Restore CPU state */
gen_helper_rfci(cpu_env);
- gen_sync_exception(ctx);
+ ctx->base.is_jmp = DISAS_EXIT;
#endif /* defined(CONFIG_USER_ONLY) */
}
@@ -6348,7 +6340,7 @@ static void gen_rfdi(DisasContext *ctx)
CHK_SV;
/* Restore CPU state */
gen_helper_rfdi(cpu_env);
- gen_sync_exception(ctx);
+ ctx->base.is_jmp = DISAS_EXIT;
#endif /* defined(CONFIG_USER_ONLY) */
}
@@ -6361,7 +6353,7 @@ static void gen_rfmci(DisasContext *ctx)
CHK_SV;
/* Restore CPU state */
gen_helper_rfmci(cpu_env);
- gen_sync_exception(ctx);
+ ctx->base.is_jmp = DISAS_EXIT;
#endif /* defined(CONFIG_USER_ONLY) */
}
@@ -8092,7 +8084,6 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
case POWERPC_EXCP_BRANCH:
ctx->base.is_jmp = DISAS_NORETURN;
break;
- case POWERPC_EXCP_SYNC:
case POWERPC_EXCP_STOP:
ctx->base.is_jmp = DISAS_EXIT;
break;
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH v3 13/30] target/ppc: Remove unnecessary gen_io_end calls
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (11 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 12/30] target/ppc: Replace POWERPC_EXCP_SYNC with DISAS_EXIT Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 1:15 ` [PATCH v3 14/30] target/ppc: Introduce gen_icount_io_start Richard Henderson
` (17 subsequent siblings)
30 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Since ba3e7926691ed33, we switched the implementation of icount
to always reset can_do_io at the start of the following TB.
Most of them were removed in 9e9b10c64911, but some were missed.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/translate_init.c.inc | 6 ------
1 file changed, 6 deletions(-)
diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index c03a7c4f52..99e5f52925 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -213,7 +213,6 @@ static void spr_read_tbl(DisasContext *ctx, int gprn, int sprn)
}
gen_helper_load_tbl(cpu_gpr[gprn], cpu_env);
if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_end();
gen_stop_exception(ctx);
}
}
@@ -225,7 +224,6 @@ static void spr_read_tbu(DisasContext *ctx, int gprn, int sprn)
}
gen_helper_load_tbu(cpu_gpr[gprn], cpu_env);
if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_end();
gen_stop_exception(ctx);
}
}
@@ -250,7 +248,6 @@ static void spr_write_tbl(DisasContext *ctx, int sprn, int gprn)
}
gen_helper_store_tbl(cpu_env, cpu_gpr[gprn]);
if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_end();
gen_stop_exception(ctx);
}
}
@@ -262,7 +259,6 @@ static void spr_write_tbu(DisasContext *ctx, int sprn, int gprn)
}
gen_helper_store_tbu(cpu_env, cpu_gpr[gprn]);
if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_end();
gen_stop_exception(ctx);
}
}
@@ -311,7 +307,6 @@ static void spr_read_hdecr(DisasContext *ctx, int gprn, int sprn)
}
gen_helper_load_hdecr(cpu_gpr[gprn], cpu_env);
if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_end();
gen_stop_exception(ctx);
}
}
@@ -323,7 +318,6 @@ static void spr_write_hdecr(DisasContext *ctx, int sprn, int gprn)
}
gen_helper_store_hdecr(cpu_env, cpu_gpr[gprn]);
if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_end();
gen_stop_exception(ctx);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH v3 14/30] target/ppc: Introduce gen_icount_io_start
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (12 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 13/30] target/ppc: Remove unnecessary gen_io_end calls Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 1:15 ` [PATCH v3 15/30] target/ppc: Replace POWERPC_EXCP_STOP with DISAS_EXIT_UPDATE Richard Henderson
` (16 subsequent siblings)
30 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Create a function to handle the details for interacting with icount.
Force the exit from the tb via DISAS_TOO_MANY, which allows chaining
to the next tb, where the code emitted for gen_tb_start() will
determine if we must exit. We can thus remove any matching
conditional call to gen_stop_exception.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/translate.c | 41 +++++-----
target/ppc/translate_init.c.inc | 133 +++++---------------------------
2 files changed, 39 insertions(+), 135 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index d22d6e5b85..45cd3189c0 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -302,6 +302,20 @@ static void gen_exception_nip(DisasContext *ctx, uint32_t excp,
ctx->base.is_jmp = DISAS_NORETURN;
}
+static void gen_icount_io_start(DisasContext *ctx)
+{
+ if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+ gen_io_start();
+ /*
+ * An I/O instruction must be last in the TB.
+ * Chain to the next TB, and let the code from gen_tb_start
+ * decide if we need to return to the main loop.
+ * Doing this first also allows this value to be overridden.
+ */
+ ctx->base.is_jmp = DISAS_TOO_MANY;
+ }
+}
+
/*
* Tells the caller what is the appropriate exception to generate and prepares
* SPR registers for this exception.
@@ -1842,18 +1856,13 @@ static void gen_darn(DisasContext *ctx)
if (l > 2) {
tcg_gen_movi_i64(cpu_gpr[rD(ctx->opcode)], -1);
} else {
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
if (l == 0) {
gen_helper_darn32(cpu_gpr[rD(ctx->opcode)]);
} else {
/* Return 64-bit random for both CRN and RRN */
gen_helper_darn64(cpu_gpr[rD(ctx->opcode)]);
}
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
}
#endif
@@ -3995,9 +4004,7 @@ static void gen_rfi(DisasContext *ctx)
}
/* Restore CPU state */
CHK_SV;
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_update_cfar(ctx, ctx->cia);
gen_helper_rfi(cpu_env);
ctx->base.is_jmp = DISAS_EXIT;
@@ -4012,9 +4019,7 @@ static void gen_rfid(DisasContext *ctx)
#else
/* Restore CPU state */
CHK_SV;
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_update_cfar(ctx, ctx->cia);
gen_helper_rfid(cpu_env);
ctx->base.is_jmp = DISAS_EXIT;
@@ -4029,9 +4034,7 @@ static void gen_rfscv(DisasContext *ctx)
#else
/* Restore CPU state */
CHK_SV;
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_update_cfar(ctx, ctx->cia);
gen_helper_rfscv(cpu_env);
ctx->base.is_jmp = DISAS_EXIT;
@@ -4406,9 +4409,7 @@ static void gen_mtmsrd(DisasContext *ctx)
CHK_SV;
#if !defined(CONFIG_USER_ONLY)
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
if (ctx->opcode & 0x00010000) {
/* L=1 form only updates EE and RI */
TCGv t0 = tcg_temp_new();
@@ -4443,9 +4444,7 @@ static void gen_mtmsr(DisasContext *ctx)
CHK_SV;
#if !defined(CONFIG_USER_ONLY)
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
if (ctx->opcode & 0x00010000) {
/* L=1 form only updates EE and RI */
TCGv t0 = tcg_temp_new();
diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index 99e5f52925..9af3fb2066 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -183,24 +183,14 @@ static void spr_write_ureg(DisasContext *ctx, int sprn, int gprn)
#if !defined(CONFIG_USER_ONLY)
static void spr_read_decr(DisasContext *ctx, int gprn, int sprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_load_decr(cpu_gpr[gprn], cpu_env);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
static void spr_write_decr(DisasContext *ctx, int sprn, int gprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_store_decr(cpu_env, cpu_gpr[gprn]);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
#endif
@@ -208,24 +198,14 @@ static void spr_write_decr(DisasContext *ctx, int sprn, int gprn)
/* Time base */
static void spr_read_tbl(DisasContext *ctx, int gprn, int sprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_load_tbl(cpu_gpr[gprn], cpu_env);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
static void spr_read_tbu(DisasContext *ctx, int gprn, int sprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_load_tbu(cpu_gpr[gprn], cpu_env);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
ATTRIBUTE_UNUSED
@@ -243,24 +223,14 @@ static void spr_read_atbu(DisasContext *ctx, int gprn, int sprn)
#if !defined(CONFIG_USER_ONLY)
static void spr_write_tbl(DisasContext *ctx, int sprn, int gprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_store_tbl(cpu_env, cpu_gpr[gprn]);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
static void spr_write_tbu(DisasContext *ctx, int sprn, int gprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_store_tbu(cpu_env, cpu_gpr[gprn]);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
ATTRIBUTE_UNUSED
@@ -279,80 +249,45 @@ static void spr_write_atbu(DisasContext *ctx, int sprn, int gprn)
ATTRIBUTE_UNUSED
static void spr_read_purr(DisasContext *ctx, int gprn, int sprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_load_purr(cpu_gpr[gprn], cpu_env);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
static void spr_write_purr(DisasContext *ctx, int sprn, int gprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_store_purr(cpu_env, cpu_gpr[gprn]);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
/* HDECR */
static void spr_read_hdecr(DisasContext *ctx, int gprn, int sprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_load_hdecr(cpu_gpr[gprn], cpu_env);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
static void spr_write_hdecr(DisasContext *ctx, int sprn, int gprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_store_hdecr(cpu_env, cpu_gpr[gprn]);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
static void spr_read_vtb(DisasContext *ctx, int gprn, int sprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_load_vtb(cpu_gpr[gprn], cpu_env);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
static void spr_write_vtb(DisasContext *ctx, int sprn, int gprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_store_vtb(cpu_env, cpu_gpr[gprn]);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
static void spr_write_tbu40(DisasContext *ctx, int sprn, int gprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_store_tbu40(cpu_env, cpu_gpr[gprn]);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
#endif
@@ -560,71 +495,41 @@ static void spr_write_601_ubatl(DisasContext *ctx, int sprn, int gprn)
#if !defined(CONFIG_USER_ONLY)
static void spr_read_40x_pit(DisasContext *ctx, int gprn, int sprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_load_40x_pit(cpu_gpr[gprn], cpu_env);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
static void spr_write_40x_pit(DisasContext *ctx, int sprn, int gprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_store_40x_pit(cpu_env, cpu_gpr[gprn]);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
static void spr_write_40x_dbcr0(DisasContext *ctx, int sprn, int gprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_store_spr(sprn, cpu_gpr[gprn]);
gen_helper_store_40x_dbcr0(cpu_env, cpu_gpr[gprn]);
/* We must stop translation as we may have rebooted */
gen_stop_exception(ctx);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
static void spr_write_40x_sler(DisasContext *ctx, int sprn, int gprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_store_40x_sler(cpu_env, cpu_gpr[gprn]);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
static void spr_write_booke_tcr(DisasContext *ctx, int sprn, int gprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_store_booke_tcr(cpu_env, cpu_gpr[gprn]);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
static void spr_write_booke_tsr(DisasContext *ctx, int sprn, int gprn)
{
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
+ gen_icount_io_start(ctx);
gen_helper_store_booke_tsr(cpu_env, cpu_gpr[gprn]);
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_stop_exception(ctx);
- }
}
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH v3 15/30] target/ppc: Replace POWERPC_EXCP_STOP with DISAS_EXIT_UPDATE
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (13 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 14/30] target/ppc: Introduce gen_icount_io_start Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 1:15 ` [PATCH v3 16/30] target/ppc: Replace POWERPC_EXCP_BRANCH with DISAS_NORETURN Richard Henderson
` (15 subsequent siblings)
30 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Remove the synthetic "exception" after no more uses.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/cpu.h | 1 -
linux-user/ppc/cpu_loop.c | 3 ---
target/ppc/translate.c | 20 +++++---------------
target/ppc/translate_init.c.inc | 4 ++--
4 files changed, 7 insertions(+), 21 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index cf10117065..9bb370abba 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -132,7 +132,6 @@ enum {
/* EOL */
POWERPC_EXCP_NB = 103,
/* QEMU exceptions: used internally during code translation */
- POWERPC_EXCP_STOP = 0x200, /* stop translation */
POWERPC_EXCP_BRANCH = 0x201, /* branch instruction */
/* QEMU exceptions: special cases we want to stop translation */
POWERPC_EXCP_SYSCALL_USER = 0x203, /* System call in user mode only */
diff --git a/linux-user/ppc/cpu_loop.c b/linux-user/ppc/cpu_loop.c
index df71e15a25..4c308835bd 100644
--- a/linux-user/ppc/cpu_loop.c
+++ b/linux-user/ppc/cpu_loop.c
@@ -423,9 +423,6 @@ void cpu_loop(CPUPPCState *env)
cpu_abort(cs, "Maintenance exception while in user mode. "
"Aborting\n");
break;
- case POWERPC_EXCP_STOP: /* stop translation */
- /* We did invalidate the instruction cache. Go on */
- break;
case POWERPC_EXCP_BRANCH: /* branch instruction: */
/* We just stopped because of a branch. Go on */
break;
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 45cd3189c0..82fdf0bb77 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -367,13 +367,6 @@ static inline void gen_hvpriv_exception(DisasContext *ctx, uint32_t error)
gen_exception_err(ctx, POWERPC_EXCP_HV_EMU, POWERPC_EXCP_PRIV | error);
}
-/* Stop translation */
-static inline void gen_stop_exception(DisasContext *ctx)
-{
- gen_update_nip(ctx, ctx->base.pc_next);
- ctx->exception = POWERPC_EXCP_STOP;
-}
-
#define GEN_HANDLER(name, opc1, opc2, opc3, inval, type) \
GEN_OPCODE(name, opc1, opc2, opc3, inval, type, PPC_NONE)
@@ -3157,7 +3150,7 @@ static void gen_isync(DisasContext *ctx)
gen_check_tlb_flush(ctx, false);
}
tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
- gen_stop_exception(ctx);
+ ctx->base.is_jmp = DISAS_EXIT_UPDATE;
}
#define MEMOP_GET_SIZE(x) (1 << ((x) & MO_SIZE))
@@ -4434,7 +4427,7 @@ static void gen_mtmsrd(DisasContext *ctx)
gen_helper_store_msr(cpu_env, cpu_gpr[rS(ctx->opcode)]);
}
/* Must stop the translation as machine state (may have) changed */
- gen_stop_exception(ctx);
+ ctx->base.is_jmp = DISAS_EXIT_UPDATE;
#endif /* !defined(CONFIG_USER_ONLY) */
}
#endif /* defined(TARGET_PPC64) */
@@ -4477,7 +4470,7 @@ static void gen_mtmsr(DisasContext *ctx)
tcg_temp_free(msr);
}
/* Must stop the translation as machine state (may have) changed */
- gen_stop_exception(ctx);
+ ctx->base.is_jmp = DISAS_EXIT_UPDATE;
#endif
}
@@ -6614,7 +6607,7 @@ static void gen_wrtee(DisasContext *ctx)
* Stop translation to have a chance to raise an exception if we
* just set msr_ee to 1
*/
- gen_stop_exception(ctx);
+ ctx->base.is_jmp = DISAS_EXIT_UPDATE;
#endif /* defined(CONFIG_USER_ONLY) */
}
@@ -6628,7 +6621,7 @@ static void gen_wrteei(DisasContext *ctx)
if (ctx->opcode & 0x00008000) {
tcg_gen_ori_tl(cpu_msr, cpu_msr, (1 << MSR_EE));
/* Stop translation to have a chance to raise an exception */
- gen_stop_exception(ctx);
+ ctx->base.is_jmp = DISAS_EXIT_UPDATE;
} else {
tcg_gen_andi_tl(cpu_msr, cpu_msr, ~(1 << MSR_EE));
}
@@ -8083,9 +8076,6 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
case POWERPC_EXCP_BRANCH:
ctx->base.is_jmp = DISAS_NORETURN;
break;
- case POWERPC_EXCP_STOP:
- ctx->base.is_jmp = DISAS_EXIT;
- break;
default:
/* Every other ctx->exception should have set NORETURN. */
g_assert_not_reached();
diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index 9af3fb2066..f3e79b8685 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -463,7 +463,7 @@ static void spr_write_hid0_601(DisasContext *ctx, int sprn, int gprn)
{
gen_helper_store_hid0_601(cpu_env, cpu_gpr[gprn]);
/* Must stop the translation as endianness may have changed */
- gen_stop_exception(ctx);
+ ctx->base.is_jmp = DISAS_EXIT_UPDATE;
}
#endif
@@ -511,7 +511,7 @@ static void spr_write_40x_dbcr0(DisasContext *ctx, int sprn, int gprn)
gen_store_spr(sprn, cpu_gpr[gprn]);
gen_helper_store_40x_dbcr0(cpu_env, cpu_gpr[gprn]);
/* We must stop translation as we may have rebooted */
- gen_stop_exception(ctx);
+ ctx->base.is_jmp = DISAS_EXIT_UPDATE;
}
static void spr_write_40x_sler(DisasContext *ctx, int sprn, int gprn)
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH v3 16/30] target/ppc: Replace POWERPC_EXCP_BRANCH with DISAS_NORETURN
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (14 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 15/30] target/ppc: Replace POWERPC_EXCP_STOP with DISAS_EXIT_UPDATE Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 1:15 ` [PATCH v3 17/30] target/ppc: Remove DisasContext.exception Richard Henderson
` (14 subsequent siblings)
30 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
The translation of branch instructions always results in exit from
the TB. Remove the synthetic "exception" after no more uses.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/cpu.h | 2 --
linux-user/ppc/cpu_loop.c | 3 ---
target/ppc/translate.c | 8 ++------
3 files changed, 2 insertions(+), 11 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 9bb370abba..5b22eb64dc 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -131,8 +131,6 @@ enum {
POWERPC_EXCP_SYSCALL_VECTORED = 102, /* scv exception */
/* EOL */
POWERPC_EXCP_NB = 103,
- /* QEMU exceptions: used internally during code translation */
- POWERPC_EXCP_BRANCH = 0x201, /* branch instruction */
/* QEMU exceptions: special cases we want to stop translation */
POWERPC_EXCP_SYSCALL_USER = 0x203, /* System call in user mode only */
};
diff --git a/linux-user/ppc/cpu_loop.c b/linux-user/ppc/cpu_loop.c
index 4c308835bd..adf5a8a4e6 100644
--- a/linux-user/ppc/cpu_loop.c
+++ b/linux-user/ppc/cpu_loop.c
@@ -423,9 +423,6 @@ void cpu_loop(CPUPPCState *env)
cpu_abort(cs, "Maintenance exception while in user mode. "
"Aborting\n");
break;
- case POWERPC_EXCP_BRANCH: /* branch instruction: */
- /* We just stopped because of a branch. Go on */
- break;
case POWERPC_EXCP_SYSCALL_USER:
/* system call in user-mode emulation */
/* WARNING:
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 82fdf0bb77..276a4a2a79 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -3769,7 +3769,6 @@ static void gen_b(DisasContext *ctx)
{
target_ulong li, target;
- ctx->exception = POWERPC_EXCP_BRANCH;
/* sign extend LI */
li = LI(ctx->opcode);
li = (li ^ 0x02000000) - 0x02000000;
@@ -3783,6 +3782,7 @@ static void gen_b(DisasContext *ctx)
}
gen_update_cfar(ctx, ctx->cia);
gen_goto_tb(ctx, 0, target);
+ ctx->base.is_jmp = DISAS_NORETURN;
}
#define BCOND_IM 0
@@ -3795,7 +3795,6 @@ static void gen_bcond(DisasContext *ctx, int type)
uint32_t bo = BO(ctx->opcode);
TCGLabel *l1;
TCGv target;
- ctx->exception = POWERPC_EXCP_BRANCH;
if (type == BCOND_LR || type == BCOND_CTR || type == BCOND_TAR) {
target = tcg_temp_local_new();
@@ -3902,6 +3901,7 @@ static void gen_bcond(DisasContext *ctx, int type)
gen_set_label(l1);
gen_goto_tb(ctx, 1, ctx->base.pc_next);
}
+ ctx->base.is_jmp = DISAS_NORETURN;
}
static void gen_bc(DisasContext *ctx)
@@ -8057,7 +8057,6 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
/* Check trace mode exceptions */
if (unlikely(ctx->singlestep_enabled & CPU_SINGLE_STEP &&
(ctx->base.pc_next <= 0x100 || ctx->base.pc_next > 0xF00) &&
- ctx->exception != POWERPC_EXCP_BRANCH &&
ctx->base.is_jmp != DISAS_NORETURN)) {
uint32_t excp = gen_prep_dbgex(ctx);
gen_exception_nip(ctx, excp, ctx->base.pc_next);
@@ -8073,9 +8072,6 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
switch (ctx->exception) {
case POWERPC_EXCP_NONE:
break;
- case POWERPC_EXCP_BRANCH:
- ctx->base.is_jmp = DISAS_NORETURN;
- break;
default:
/* Every other ctx->exception should have set NORETURN. */
g_assert_not_reached();
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH v3 17/30] target/ppc: Remove DisasContext.exception
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (15 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 16/30] target/ppc: Replace POWERPC_EXCP_BRANCH with DISAS_NORETURN Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 13:00 ` Matheus K. Ferst
2021-04-30 1:15 ` [PATCH v3 18/30] target/ppc: Move single-step check to ppc_tr_tb_stop Richard Henderson
` (13 subsequent siblings)
30 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Now that we have removed all of the fake exceptions, and all real
exceptions exit via DISAS_NORETURN, we can remove this field.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/translate.c | 22 ++--------------------
1 file changed, 2 insertions(+), 20 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 276a4a2a79..d78071a4a4 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -259,15 +259,12 @@ static void gen_exception_err(DisasContext *ctx, uint32_t excp, uint32_t error)
* These are all synchronous exceptions, we set the PC back to the
* faulting instruction
*/
- if (ctx->exception == POWERPC_EXCP_NONE) {
- gen_update_nip(ctx, ctx->cia);
- }
+ gen_update_nip(ctx, ctx->cia);
t0 = tcg_const_i32(excp);
t1 = tcg_const_i32(error);
gen_helper_raise_exception_err(cpu_env, t0, t1);
tcg_temp_free_i32(t0);
tcg_temp_free_i32(t1);
- ctx->exception = excp;
ctx->base.is_jmp = DISAS_NORETURN;
}
@@ -279,13 +276,10 @@ static void gen_exception(DisasContext *ctx, uint32_t excp)
* These are all synchronous exceptions, we set the PC back to the
* faulting instruction
*/
- if (ctx->exception == POWERPC_EXCP_NONE) {
- gen_update_nip(ctx, ctx->cia);
- }
+ gen_update_nip(ctx, ctx->cia);
t0 = tcg_const_i32(excp);
gen_helper_raise_exception(cpu_env, t0);
tcg_temp_free_i32(t0);
- ctx->exception = excp;
ctx->base.is_jmp = DISAS_NORETURN;
}
@@ -298,7 +292,6 @@ static void gen_exception_nip(DisasContext *ctx, uint32_t excp,
t0 = tcg_const_i32(excp);
gen_helper_raise_exception(cpu_env, t0);
tcg_temp_free_i32(t0);
- ctx->exception = excp;
ctx->base.is_jmp = DISAS_NORETURN;
}
@@ -7919,7 +7912,6 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
CPUPPCState *env = cs->env_ptr;
int bound;
- ctx->exception = POWERPC_EXCP_NONE;
ctx->spr_cb = env->spr_cb;
ctx->pr = msr_pr;
ctx->mem_idx = env->dmmu_idx;
@@ -8067,16 +8059,6 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
"temporaries\n", opc1(ctx->opcode), opc2(ctx->opcode),
opc3(ctx->opcode), opc4(ctx->opcode), ctx->opcode);
}
-
- if (ctx->base.is_jmp == DISAS_NEXT) {
- switch (ctx->exception) {
- case POWERPC_EXCP_NONE:
- break;
- default:
- /* Every other ctx->exception should have set NORETURN. */
- g_assert_not_reached();
- }
- }
}
static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH v3 17/30] target/ppc: Remove DisasContext.exception
2021-04-30 1:15 ` [PATCH v3 17/30] target/ppc: Remove DisasContext.exception Richard Henderson
@ 2021-04-30 13:00 ` Matheus K. Ferst
0 siblings, 0 replies; 63+ messages in thread
From: Matheus K. Ferst @ 2021-04-30 13:00 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen, david
On 29/04/2021 22:15, Richard Henderson wrote:
> Now that we have removed all of the fake exceptions, and all real
> exceptions exit via DISAS_NORETURN, we can remove this field.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/ppc/translate.c | 22 ++--------------------
> 1 file changed, 2 insertions(+), 20 deletions(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 276a4a2a79..d78071a4a4 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -259,15 +259,12 @@ static void gen_exception_err(DisasContext *ctx, uint32_t excp, uint32_t error)
> * These are all synchronous exceptions, we set the PC back to the
> * faulting instruction
> */
> - if (ctx->exception == POWERPC_EXCP_NONE) {
> - gen_update_nip(ctx, ctx->cia);
> - }
> + gen_update_nip(ctx, ctx->cia);
> t0 = tcg_const_i32(excp);
> t1 = tcg_const_i32(error);
> gen_helper_raise_exception_err(cpu_env, t0, t1);
> tcg_temp_free_i32(t0);
> tcg_temp_free_i32(t1);
> - ctx->exception = excp;
> ctx->base.is_jmp = DISAS_NORETURN;
> }
>
> @@ -279,13 +276,10 @@ static void gen_exception(DisasContext *ctx, uint32_t excp)
> * These are all synchronous exceptions, we set the PC back to the
> * faulting instruction
> */
> - if (ctx->exception == POWERPC_EXCP_NONE) {
> - gen_update_nip(ctx, ctx->cia);
> - }
> + gen_update_nip(ctx, ctx->cia);
> t0 = tcg_const_i32(excp);
> gen_helper_raise_exception(cpu_env, t0);
> tcg_temp_free_i32(t0);
> - ctx->exception = excp;
> ctx->base.is_jmp = DISAS_NORETURN;
> }
>
> @@ -298,7 +292,6 @@ static void gen_exception_nip(DisasContext *ctx, uint32_t excp,
> t0 = tcg_const_i32(excp);
> gen_helper_raise_exception(cpu_env, t0);
> tcg_temp_free_i32(t0);
> - ctx->exception = excp;
> ctx->base.is_jmp = DISAS_NORETURN;
> }
>
> @@ -7919,7 +7912,6 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> CPUPPCState *env = cs->env_ptr;
> int bound;
>
> - ctx->exception = POWERPC_EXCP_NONE;
> ctx->spr_cb = env->spr_cb;
> ctx->pr = msr_pr;
> ctx->mem_idx = env->dmmu_idx;
> @@ -8067,16 +8059,6 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
> "temporaries\n", opc1(ctx->opcode), opc2(ctx->opcode),
> opc3(ctx->opcode), opc4(ctx->opcode), ctx->opcode);
> }
> -
> - if (ctx->base.is_jmp == DISAS_NEXT) {
> - switch (ctx->exception) {
> - case POWERPC_EXCP_NONE:
> - break;
> - default:
> - /* Every other ctx->exception should have set NORETURN. */
> - g_assert_not_reached();
> - }
> - }
> }
>
> static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
>
You removed the uses, but left the field.
Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software Júnior
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH v3 18/30] target/ppc: Move single-step check to ppc_tr_tb_stop
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (16 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 17/30] target/ppc: Remove DisasContext.exception Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 1:15 ` [PATCH v3 19/30] target/ppc: Tidy exception vs exit_tb Richard Henderson
` (12 subsequent siblings)
30 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
When single-stepping, force max_insns to 1 in init_disas
so that we exit the translation loop immediately.
Combine the single-step checks in tb_stop, and give the
gdb exception priority over the cpu exception, just as
we already do in gen_lookup_and_goto_ptr.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/translate.c | 37 +++++++++++++++++++------------------
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index d78071a4a4..c018960ce9 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7910,7 +7910,6 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
{
DisasContext *ctx = container_of(dcbase, DisasContext, base);
CPUPPCState *env = cs->env_ptr;
- int bound;
ctx->spr_cb = env->spr_cb;
ctx->pr = msr_pr;
@@ -7986,13 +7985,13 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
if (unlikely(ctx->base.singlestep_enabled)) {
ctx->singlestep_enabled |= GDBSTUB_SINGLE_STEP;
}
-#if defined(DO_SINGLE_STEP) && 0
- /* Single step trace mode */
- msr_se = 1;
-#endif
- bound = -(ctx->base.pc_first | TARGET_PAGE_MASK) / 4;
- ctx->base.max_insns = MIN(ctx->base.max_insns, bound);
+ if (ctx->singlestep_enabled & (CPU_SINGLE_STEP | GDBSTUB_SINGLE_STEP)) {
+ ctx->base.max_insns = 1;
+ } else {
+ int bound = -(ctx->base.pc_first | TARGET_PAGE_MASK) / 4;
+ ctx->base.max_insns = MIN(ctx->base.max_insns, bound);
+ }
}
static void ppc_tr_tb_start(DisasContextBase *db, CPUState *cs)
@@ -8046,14 +8045,6 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
handler->count++;
#endif
- /* Check trace mode exceptions */
- if (unlikely(ctx->singlestep_enabled & CPU_SINGLE_STEP &&
- (ctx->base.pc_next <= 0x100 || ctx->base.pc_next > 0xF00) &&
- ctx->base.is_jmp != DISAS_NORETURN)) {
- uint32_t excp = gen_prep_dbgex(ctx);
- gen_exception_nip(ctx, excp, ctx->base.pc_next);
- }
-
if (tcg_check_temp_count()) {
qemu_log("Opcode %02x %02x %02x %02x (%08x) leaked "
"temporaries\n", opc1(ctx->opcode), opc2(ctx->opcode),
@@ -8066,6 +8057,7 @@ static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
DisasContext *ctx = container_of(dcbase, DisasContext, base);
DisasJumpType is_jmp = ctx->base.is_jmp;
target_ulong nip = ctx->base.pc_next;
+ int sse;
if (is_jmp == DISAS_NORETURN) {
/* We have already exited the TB. */
@@ -8073,7 +8065,8 @@ static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
/* Honor single stepping. */
- if (unlikely(ctx->base.singlestep_enabled)) {
+ sse = ctx->singlestep_enabled & (CPU_SINGLE_STEP | GDBSTUB_SINGLE_STEP);
+ if (unlikely(sse)) {
switch (is_jmp) {
case DISAS_TOO_MANY:
case DISAS_EXIT_UPDATE:
@@ -8086,8 +8079,16 @@ static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
default:
g_assert_not_reached();
}
- gen_debug_exception(ctx);
- return;
+
+ if (sse & GDBSTUB_SINGLE_STEP) {
+ gen_debug_exception(ctx);
+ return;
+ }
+ /* else CPU_SINGLE_STEP... */
+ if (nip <= 0x100 || nip > 0xf00) {
+ gen_exception(ctx, gen_prep_dbgex(ctx));
+ return;
+ }
}
switch (is_jmp) {
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH v3 19/30] target/ppc: Tidy exception vs exit_tb
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (17 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 18/30] target/ppc: Move single-step check to ppc_tr_tb_stop Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 1:15 ` [PATCH v3 20/30] target/ppc: Mark helper_raise_exception* as noreturn Richard Henderson
` (11 subsequent siblings)
30 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
We do not need to emit an exit_tb after an exception,
as the latter will exit via longjmp.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/translate.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index c018960ce9..fe3982e289 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -3726,8 +3726,9 @@ static void gen_lookup_and_goto_ptr(DisasContext *ctx)
} else if (sse & (CPU_SINGLE_STEP | CPU_BRANCH_STEP)) {
uint32_t excp = gen_prep_dbgex(ctx);
gen_exception(ctx, excp);
+ } else {
+ tcg_gen_exit_tb(NULL, 0);
}
- tcg_gen_exit_tb(NULL, 0);
} else {
tcg_gen_lookup_and_goto_ptr();
}
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH v3 20/30] target/ppc: Mark helper_raise_exception* as noreturn
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (18 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 19/30] target/ppc: Tidy exception vs exit_tb Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-05-03 22:36 ` Philippe Mathieu-Daudé
2021-04-30 1:15 ` [PATCH v3 21/30] target/ppc: Use translator_loop_temp_check Richard Henderson
` (10 subsequent siblings)
30 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/helper.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 6a4dccf70c..af5b3586d1 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -1,5 +1,5 @@
-DEF_HELPER_FLAGS_3(raise_exception_err, TCG_CALL_NO_WG, void, env, i32, i32)
-DEF_HELPER_FLAGS_2(raise_exception, TCG_CALL_NO_WG, void, env, i32)
+DEF_HELPER_FLAGS_3(raise_exception_err, TCG_CALL_NO_WG, noreturn, env, i32, i32)
+DEF_HELPER_FLAGS_2(raise_exception, TCG_CALL_NO_WG, noreturn, env, i32)
DEF_HELPER_FLAGS_4(tw, TCG_CALL_NO_WG, void, env, tl, tl, i32)
#if defined(TARGET_PPC64)
DEF_HELPER_FLAGS_4(td, TCG_CALL_NO_WG, void, env, tl, tl, i32)
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH v3 20/30] target/ppc: Mark helper_raise_exception* as noreturn
2021-04-30 1:15 ` [PATCH v3 20/30] target/ppc: Mark helper_raise_exception* as noreturn Richard Henderson
@ 2021-05-03 22:36 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 63+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-03 22:36 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: luis.pires, qemu-ppc, lagarcia, bruno.larsen, matheus.ferst, david
On 4/30/21 3:15 AM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/ppc/helper.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH v3 21/30] target/ppc: Use translator_loop_temp_check
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (19 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 20/30] target/ppc: Mark helper_raise_exception* as noreturn Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 1:15 ` [PATCH v3 22/30] target/ppc: Introduce macros to check isa extensions Richard Henderson
` (9 subsequent siblings)
30 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
The special logging is unnecessary. It will have been done
immediately before in the log file.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/translate.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index fe3982e289..112afd02d5 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -8046,11 +8046,7 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
handler->count++;
#endif
- if (tcg_check_temp_count()) {
- qemu_log("Opcode %02x %02x %02x %02x (%08x) leaked "
- "temporaries\n", opc1(ctx->opcode), opc2(ctx->opcode),
- opc3(ctx->opcode), opc4(ctx->opcode), ctx->opcode);
- }
+ translator_loop_temp_check(&ctx->base);
}
static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH v3 22/30] target/ppc: Introduce macros to check isa extensions
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (20 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 21/30] target/ppc: Use translator_loop_temp_check Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-05-03 22:37 ` Philippe Mathieu-Daudé
2021-04-30 1:15 ` [PATCH v3 23/30] target/ppc: Add infrastructure for prefixed insns Richard Henderson
` (8 subsequent siblings)
30 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
These will be used by the decodetree trans_* functions
to early-exit when the instruction set is not enabled.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/translate.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 112afd02d5..cde12e9d38 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6876,6 +6876,32 @@ static inline void set_avr64(int regno, TCGv_i64 src, bool high)
tcg_gen_st_i64(src, cpu_env, avr64_offset(regno, high));
}
+/*
+ * Helpers for trans_* functions to check for specific insns flags.
+ * Use token pasting to ensure that we use the proper flag with the
+ * proper variable.
+ */
+#define REQUIRE_INSNS_FLAGS(CTX, NAME) \
+ do { \
+ if (((CTX)->insns_flags & PPC_##NAME) == 0) { \
+ return false; \
+ } \
+ } while (0)
+
+#define REQUIRE_INSNS_FLAGS2(CTX, NAME) \
+ do { \
+ if (((CTX)->insns_flags2 & PPC2_##NAME) == 0) { \
+ return false; \
+ } \
+ } while (0)
+
+/* Then special-case the check for 64-bit so that we elide code for ppc32. */
+#if TARGET_LONG_BITS == 32
+# define REQUIRE_64BIT(CTX) return false
+#else
+# define REQUIRE_64BIT(CTX) REQUIRE_INSNS_FLAGS(CTX, 64B)
+#endif
+
#include "translate/fp-impl.c.inc"
#include "translate/vmx-impl.c.inc"
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH v3 23/30] target/ppc: Add infrastructure for prefixed insns
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (21 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 22/30] target/ppc: Introduce macros to check isa extensions Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 1:15 ` [PATCH v3 24/30] target/ppc: Move page crossing check to ppc_tr_translate_insn Richard Henderson
` (7 subsequent siblings)
30 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Signed-off-by: Luis Pires <luis.pires@eldorado.org.br>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v3: Move page crossing check to its own patch,
fold in ISA310 check to is_prefix_insn
---
target/ppc/cpu.h | 1 +
target/ppc/insn32.decode | 18 ++++++++++++
target/ppc/insn64.decode | 18 ++++++++++++
target/ppc/translate.c | 34 +++++++++++++++++++---
target/ppc/translate/fixedpoint-impl.c.inc | 18 ++++++++++++
target/ppc/meson.build | 9 ++++++
6 files changed, 94 insertions(+), 4 deletions(-)
create mode 100644 target/ppc/insn32.decode
create mode 100644 target/ppc/insn64.decode
create mode 100644 target/ppc/translate/fixedpoint-impl.c.inc
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 5b22eb64dc..82a2bf1e58 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -144,6 +144,7 @@ enum {
POWERPC_EXCP_ALIGN_PROT = 0x04, /* Access cross protection boundary */
POWERPC_EXCP_ALIGN_BAT = 0x05, /* Access cross a BAT/seg boundary */
POWERPC_EXCP_ALIGN_CACHE = 0x06, /* Impossible dcbz access */
+ POWERPC_EXCP_ALIGN_INSN = 0x07, /* Pref. insn x-ing 64-byte boundary */
/* Exception subtypes for POWERPC_EXCP_PROGRAM */
/* FP exceptions */
POWERPC_EXCP_FP = 0x10,
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
new file mode 100644
index 0000000000..b175441209
--- /dev/null
+++ b/target/ppc/insn32.decode
@@ -0,0 +1,18 @@
+#
+# Power ISA decode for 32-bit insns (opcode space 0)
+#
+# Copyright (c) 2021 Luis Pires <luis.pires@eldorado.org.br>
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, see <http://www.gnu.org/licenses/>.
+#
diff --git a/target/ppc/insn64.decode b/target/ppc/insn64.decode
new file mode 100644
index 0000000000..9fc45d0614
--- /dev/null
+++ b/target/ppc/insn64.decode
@@ -0,0 +1,18 @@
+#
+# Power ISA decode for 64-bit prefixed insns (opcode space 0 and 1)
+#
+# Copyright (c) 2021 Luis Pires <luis.pires@eldorado.org.br>
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, see <http://www.gnu.org/licenses/>.
+#
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index cde12e9d38..65848463ea 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6902,6 +6902,10 @@ static inline void set_avr64(int regno, TCGv_i64 src, bool high)
# define REQUIRE_64BIT(CTX) REQUIRE_INSNS_FLAGS(CTX, 64B)
#endif
+#include "decode-insn32.c.inc"
+#include "decode-insn64.c.inc"
+#include "translate/fixedpoint-impl.c.inc"
+
#include "translate/fp-impl.c.inc"
#include "translate/vmx-impl.c.inc"
@@ -8047,11 +8051,18 @@ static bool ppc_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
return true;
}
+static bool is_prefix_insn(DisasContext *ctx, uint32_t insn)
+{
+ REQUIRE_INSNS_FLAGS2(ctx, ISA310);
+ return opc1(insn) == 1;
+}
+
static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
{
DisasContext *ctx = container_of(dcbase, DisasContext, base);
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = cs->env_ptr;
+ target_ulong pc;
uint32_t insn;
bool ok;
@@ -8059,11 +8070,26 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
LOG_DISAS("nip=" TARGET_FMT_lx " super=%d ir=%d\n",
ctx->base.pc_next, ctx->mem_idx, (int)msr_ir);
- ctx->cia = ctx->base.pc_next;
- insn = translator_ldl_swap(env, ctx->base.pc_next, need_byteswap(ctx));
- ctx->base.pc_next += 4;
+ ctx->cia = pc = ctx->base.pc_next;
+ insn = translator_ldl_swap(env, pc, need_byteswap(ctx));
+ ctx->base.pc_next = pc += 4;
- ok = decode_legacy(cpu, ctx, insn);
+ if (!is_prefix_insn(ctx, insn)) {
+ ok = (decode_insn32(ctx, insn) ||
+ decode_legacy(cpu, ctx, insn));
+ } else if ((pc & 63) == 0) {
+ /*
+ * Power v3.1, section 1.9 Exceptions:
+ * attempt to execute a prefixed instruction that crosses a
+ * 64-byte address boundary (system alignment error).
+ */
+ gen_exception_err(ctx, POWERPC_EXCP_ALIGN, POWERPC_EXCP_ALIGN_INSN);
+ ok = true;
+ } else {
+ uint32_t insn2 = translator_ldl_swap(env, pc, need_byteswap(ctx));
+ ctx->base.pc_next = pc += 4;
+ ok = decode_insn64(ctx, deposit64(insn2, 32, 32, insn));
+ }
if (!ok) {
gen_invalid(ctx);
}
diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc
new file mode 100644
index 0000000000..b740083605
--- /dev/null
+++ b/target/ppc/translate/fixedpoint-impl.c.inc
@@ -0,0 +1,18 @@
+/*
+ * Power ISA decode for Fixed-Point Facility instructions
+ *
+ * Copyright (c) 2021 Luis Pires <luis.pires@eldorado.org.br>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
diff --git a/target/ppc/meson.build b/target/ppc/meson.build
index bbfef90e08..e604e56c6a 100644
--- a/target/ppc/meson.build
+++ b/target/ppc/meson.build
@@ -15,6 +15,15 @@ ppc_ss.add(files(
ppc_ss.add(libdecnumber)
+gen = [
+ decodetree.process('insn32.decode',
+ extra_args: '--static-decode=decode_insn32'),
+ decodetree.process('insn64.decode',
+ extra_args: ['--static-decode=decode_insn64',
+ '--insnwidth=64']),
+]
+ppc_ss.add(gen)
+
ppc_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c'), if_false: files('kvm-stub.c'))
ppc_ss.add(when: 'CONFIG_USER_ONLY', if_true: files('user_only_helper.c'))
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH v3 24/30] target/ppc: Move page crossing check to ppc_tr_translate_insn
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (22 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 23/30] target/ppc: Add infrastructure for prefixed insns Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 1:26 ` Richard Henderson
2021-04-30 1:15 ` [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI Richard Henderson
` (6 subsequent siblings)
30 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
With prefixed instructions, the number of instructions
remaining until the page crossing is no longer constant.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/translate.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 65848463ea..d782a13d27 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -8019,9 +8019,6 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
if (ctx->singlestep_enabled & (CPU_SINGLE_STEP | GDBSTUB_SINGLE_STEP)) {
ctx->base.max_insns = 1;
- } else {
- int bound = -(ctx->base.pc_first | TARGET_PAGE_MASK) / 4;
- ctx->base.max_insns = MIN(ctx->base.max_insns, bound);
}
}
@@ -8098,6 +8095,11 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
handler->count++;
#endif
+ /* End the TB when crossing a page boundary. */
+ if (ctx->base.is_jmp == DISAS_NEXT && !(pc & ~TARGET_PAGE_MASK)) {
+ ctx->base.is_jmp = DISAS_TOO_MANY;
+ }
+
translator_loop_temp_check(&ctx->base);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH v3 24/30] target/ppc: Move page crossing check to ppc_tr_translate_insn
2021-04-30 1:15 ` [PATCH v3 24/30] target/ppc: Move page crossing check to ppc_tr_translate_insn Richard Henderson
@ 2021-04-30 1:26 ` Richard Henderson
0 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:26 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
On 4/29/21 6:15 PM, Richard Henderson wrote:
> With prefixed instructions, the number of instructions
> remaining until the page crossing is no longer constant.
>
> Signed-off-by: Richard Henderson<richard.henderson@linaro.org>
> ---
> target/ppc/translate.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
Oops, this was supposed to be ordered before the previous patch.
r~
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (23 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 24/30] target/ppc: Move page crossing check to ppc_tr_translate_insn Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 11:23 ` Luis Fernando Fujita Pires
2021-04-30 14:05 ` Matheus K. Ferst
2021-04-30 1:15 ` [PATCH v3 26/30] target/ppc: Implement PNOP Richard Henderson
` (5 subsequent siblings)
30 siblings, 2 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/insn32.decode | 12 +++++++
target/ppc/insn64.decode | 15 +++++++++
target/ppc/translate.c | 29 ----------------
target/ppc/translate/fixedpoint-impl.c.inc | 39 ++++++++++++++++++++++
4 files changed, 66 insertions(+), 29 deletions(-)
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index b175441209..52d9b355d4 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -16,3 +16,15 @@
# You should have received a copy of the GNU Lesser General Public
# License along with this library; if not, see <http://www.gnu.org/licenses/>.
#
+
+&D rt ra si
+@D ...... rt:5 ra:5 si:s16 &D
+
+# If a prefix is allowed, decode with default values.
+&PLS_D rt ra si:int64_t r:bool
+@PLS_D ...... rt:5 ra:5 si:s16 &PLS_D r=0
+
+### Fixed-Point Arithmetic Instructions
+
+ADDI 001110 ..... ..... ................ @PLS_D
+ADDIS 001111 ..... ..... ................ @D
diff --git a/target/ppc/insn64.decode b/target/ppc/insn64.decode
index 9fc45d0614..f4272df724 100644
--- a/target/ppc/insn64.decode
+++ b/target/ppc/insn64.decode
@@ -16,3 +16,18 @@
# You should have received a copy of the GNU Lesser General Public
# License along with this library; if not, see <http://www.gnu.org/licenses/>.
#
+
+# Many all of these instruction names would be prefixed by "P",
+# but we share code with the non-prefixed instruction.
+
+# Format MLS:D and 8LS:D
+&PLS_D rt ra si:int64_t r:bool !extern
+%pls_si 32:s18 0:16
+@PLS_D ...... .. ... r:1 .. .................. \
+ ...... rt:5 ra:5 ................ \
+ &PLS_D si=%pls_si
+
+### Fixed-Point Arithmetic Instructions
+
+ADDI 000001 10 0--.-- .................. \
+ 001110 ..... ..... ................ @PLS_D
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index d782a13d27..5a8a3c39c3 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -924,19 +924,6 @@ GEN_INT_ARITH_ADD(addex, 0x05, cpu_ov, 1, 1, 0);
/* addze addze. addzeo addzeo.*/
GEN_INT_ARITH_ADD_CONST(addze, 0x06, 0, cpu_ca, 1, 1, 0)
GEN_INT_ARITH_ADD_CONST(addzeo, 0x16, 0, cpu_ca, 1, 1, 1)
-/* addi */
-static void gen_addi(DisasContext *ctx)
-{
- target_long simm = SIMM(ctx->opcode);
-
- if (rA(ctx->opcode) == 0) {
- /* li case */
- tcg_gen_movi_tl(cpu_gpr[rD(ctx->opcode)], simm);
- } else {
- tcg_gen_addi_tl(cpu_gpr[rD(ctx->opcode)],
- cpu_gpr[rA(ctx->opcode)], simm);
- }
-}
/* addic addic.*/
static inline void gen_op_addic(DisasContext *ctx, bool compute_rc0)
{
@@ -956,20 +943,6 @@ static void gen_addic_(DisasContext *ctx)
gen_op_addic(ctx, 1);
}
-/* addis */
-static void gen_addis(DisasContext *ctx)
-{
- target_long simm = SIMM(ctx->opcode);
-
- if (rA(ctx->opcode) == 0) {
- /* lis case */
- tcg_gen_movi_tl(cpu_gpr[rD(ctx->opcode)], simm << 16);
- } else {
- tcg_gen_addi_tl(cpu_gpr[rD(ctx->opcode)],
- cpu_gpr[rA(ctx->opcode)], simm << 16);
- }
-}
-
/* addpcis */
static void gen_addpcis(DisasContext *ctx)
{
@@ -7029,10 +7002,8 @@ GEN_HANDLER_E(cmpeqb, 0x1F, 0x00, 0x07, 0x00600000, PPC_NONE, PPC2_ISA300),
GEN_HANDLER_E(cmpb, 0x1F, 0x1C, 0x0F, 0x00000001, PPC_NONE, PPC2_ISA205),
GEN_HANDLER_E(cmprb, 0x1F, 0x00, 0x06, 0x00400001, PPC_NONE, PPC2_ISA300),
GEN_HANDLER(isel, 0x1F, 0x0F, 0xFF, 0x00000001, PPC_ISEL),
-GEN_HANDLER(addi, 0x0E, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
GEN_HANDLER(addic, 0x0C, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
GEN_HANDLER2(addic_, "addic.", 0x0D, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
-GEN_HANDLER(addis, 0x0F, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
GEN_HANDLER_E(addpcis, 0x13, 0x2, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA300),
GEN_HANDLER(mulhw, 0x1F, 0x0B, 0x02, 0x00000400, PPC_INTEGER),
GEN_HANDLER(mulhwu, 0x1F, 0x0B, 0x00, 0x00000400, PPC_INTEGER),
diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc
index b740083605..7af1b3bcf5 100644
--- a/target/ppc/translate/fixedpoint-impl.c.inc
+++ b/target/ppc/translate/fixedpoint-impl.c.inc
@@ -16,3 +16,42 @@
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, see <http://www.gnu.org/licenses/>.
*/
+
+/*
+ * Incorporate CIA into the constant when R=1.
+ * Validate that when R=1, RA=0.
+ */
+static bool resolve_PLS_D(DisasContext *ctx, arg_PLS_D *a)
+{
+ if (a->r) {
+ if (unlikely(a->ra != 0)) {
+ gen_invalid(ctx);
+ return false;
+ }
+ a->si += ctx->cia;
+ }
+ return true;
+}
+
+static bool trans_ADDI(DisasContext *ctx, arg_PLS_D *a)
+{
+ if (resolve_PLS_D(ctx, a)) {
+ if (a->ra) {
+ tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_gpr[a->ra], a->si);
+ } else {
+ tcg_gen_movi_tl(cpu_gpr[a->rt], a->si);
+ }
+ }
+ return true;
+}
+
+static bool trans_ADDIS(DisasContext *ctx, arg_D *a)
+{
+ int si = a->si << 16;
+ if (a->ra) {
+ tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_gpr[a->ra], si);
+ } else {
+ tcg_gen_movi_tl(cpu_gpr[a->rt], si);
+ }
+ return true;
+}
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* RE: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI
2021-04-30 1:15 ` [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI Richard Henderson
@ 2021-04-30 11:23 ` Luis Fernando Fujita Pires
2021-04-30 14:23 ` Richard Henderson
2021-04-30 14:05 ` Matheus K. Ferst
1 sibling, 1 reply; 63+ messages in thread
From: Luis Fernando Fujita Pires @ 2021-04-30 11:23 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: f4bug, qemu-ppc, lagarcia, Bruno Piazera Larsen,
Matheus Kowalczuk Ferst, david
From: Richard Henderson <richard.henderson@linaro.org>
> +&D rt ra si
> +@D ...... rt:5 ra:5 si:s16 &D
> +
> +# If a prefix is allowed, decode with default values.
> +&PLS_D rt ra si:int64_t r:bool
> +@PLS_D ...... rt:5 ra:5 si:s16 &PLS_D r=0
> +
> +### Fixed-Point Arithmetic Instructions
> +
> +ADDI 001110 ..... ..... ................ @PLS_D
> +ADDIS 001111 ..... ..... ................ @D
> diff --git a/target/ppc/insn64.decode b/target/ppc/insn64.decode index
> 9fc45d0614..f4272df724 100644
> --- a/target/ppc/insn64.decode
> +++ b/target/ppc/insn64.decode
> @@ -16,3 +16,18 @@
> # You should have received a copy of the GNU Lesser General Public # License
> along with this library; if not, see <http://www.gnu.org/licenses/>.
> #
> +
> +# Many all of these instruction names would be prefixed by "P", # but
> +we share code with the non-prefixed instruction.
> +
> +# Format MLS:D and 8LS:D
> +&PLS_D rt ra si:int64_t r:bool !extern
> +%pls_si 32:s18 0:16
> +@PLS_D ...... .. ... r:1 .. .................. \
> + ...... rt:5 ra:5 ................ \
> + &PLS_D si=%pls_si
> +
> +### Fixed-Point Arithmetic Instructions
> +
> +ADDI 000001 10 0--.-- .................. \
> + 001110 ..... ..... ................ @PLS_D
I think we should reconsider using the same .decode file for both 32- and 64-bit instructions, to avoid duplicating argument set definitions, and to keep the prefixed instructions close to their non-prefixed counterparts. For ADDI/PADDI, something along the lines of:
&PLS_D rt ra si:int64_t r:bool
%pls_si 32:s18 0:16
@PLS_D ...... .. ... r:1 .. .................. \
...... rt:5 ra:5 ................ \
&PLS_D si=%pls_si
@PLS_D_32 ...... rt:5 ra:5 si:s16 &PLS_D r=0
PADDI 000001 10 0--.-- .................. \
001110 ..... ..... ................ @PLS_D
ADDI 001110 ..... ..... ................ @PLS_D_32
ADDIS 001111 ..... ..... ................ @D
That's where I was going with the original patch, using the varinsnwidth support from decodetree.py.
And, in order to share the trans_PADDI/ADDI implementation, maybe add something to decodetree.py to allow us to specify that an instruction shares the trans_XX() implementation from another one, such as:
ADDI 001110 ..... ..... ................ @PLS_D_32 !impl=PADDI
This way, we could (and would need to, in fact) keep the 'P' in the prefixed instruction names, but at the same time avoid having extra trans_XX functions just calling another one without any additional code.
For the load functions, we would then have:
%ds_si 2:s14 !function=times_4
@PLS_DS_32 ...... rt:5 ra:5 .............. .. &PLS_D si=%ds_si r=0
&X rt ra rb
@X ...... rt:5 ra:5 rb:5 .......... . &X
PLBZ 000001 10 0--.-- .................. \
100010 ..... ..... ................ @PLS_D
LBZ 100010 ..... ..... ................ @PLS_D_32 !impl=PLBZ
LBZU 100011 ..... ..... ................ @PLS_D_32
LBZX 011111 ..... ..... ..... 0001010111 - @X
LBZUX 011111 ..... ..... ..... 0001110111 - @X
PLHZ 000001 10 0--.-- .................. \
101000 ..... ..... ................ @PLS_D
LHZ 101000 ..... ..... ................ @PLS_D_32 !impl=PLHZ
LHZU 101001 ..... ..... ................ @PLS_D_32
LHZX 011111 ..... ..... ..... 0100010111 - @X
LHZUX 011111 ..... ..... ..... 0100110111 - @X
PLHA 000001 10 0--.-- .................. \
101010 ..... ..... ................ @PLS_D
LHA 101010 ..... ..... ................ @PLS_D_32 !impl=PLHA
LHAU 101011 ..... ..... ................ @PLS_D_32
LHAX 011111 ..... ..... ..... 0101010111 - @X
LHAXU 011111 ..... ..... ..... 0101110111 - @X
PLWZ 000001 10 0--.-- .................. \
100000 ..... ..... ................ @PLS_D
LWZ 100000 ..... ..... ................ @PLS_D_32 !impl=PLWZ
LWZU 100001 ..... ..... ................ @PLS_D_32
LWZX 011111 ..... ..... ..... 0000010111 - @X
LWZUX 011111 ..... ..... ..... 0000110111 - @X
PLWA 000001 00 0--.-- .................. \
101001 ..... ..... ................ @PLS_D
LWA 111010 ..... ..... ..............10 @PLS_DS_32 !impl=PLWA
LWAX 011111 ..... ..... ..... 0101010101 - @X
LWAUX 011111 ..... ..... ..... 0101110101 - @X
PLD 000001 00 0--.-- .................. \
111001 ..... ..... ................ @PLS_D
LD 111010 ..... ..... ..............00 @PLS_DS_32 !impl=PLD
LDU 111010 ..... ..... ..............01 @PLS_DS_32
LDX 011111 ..... ..... ..... 0000010101 - @X
LDUX 011111 ..... ..... ..... 0000110101 - @X
--
Luis
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI
2021-04-30 11:23 ` Luis Fernando Fujita Pires
@ 2021-04-30 14:23 ` Richard Henderson
2021-04-30 18:45 ` Luis Fernando Fujita Pires
0 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 14:23 UTC (permalink / raw)
To: Luis Fernando Fujita Pires, qemu-devel
Cc: f4bug, qemu-ppc, lagarcia, Bruno Piazera Larsen,
Matheus Kowalczuk Ferst, david
On 4/30/21 4:23 AM, Luis Fernando Fujita Pires wrote:
> I think we should reconsider using the same .decode file for both 32- and
> 64-bit instructions, to avoid duplicating argument set definitions, and to
> keep the prefixed instructions close to their non-prefixed counterparts.
varinsnwidth assumes there is no easy way to determine, before decoding, the
width of the instruction. The way this is implemented in decodetree is vastly
less optimal than what we can do with a few lines for ppc.
In addition, there's a rough spot in %field definitions. You can't share those
between patterns of different sizes, which can get confusing. Have a look at
target/rx, and the definitions of %b[23]_r_0, which is the same field for 2 and
3-byte insns.
The replication of argument set definitions is unfortunate, but in the end will
only be a handful of lines. We could probably come up with a way to avoid that
too, via a decodetree extension, if you really insist. (My vague idea there
would put the argument set definitions into a 3rd file, included on the
decodetree command-line.)
> And, in order to share the trans_PADDI/ADDI implementation, maybe add something to decodetree.py to allow us to specify that an instruction shares the trans_XX() implementation from another one, such as:
> ADDI 001110 ..... ..... ................ @PLS_D_32 !impl=PADDI
This is done by using the same name up front.
If you like, add a comment to give the real instruction name.
PADDI 001110 ..... ..... ................ @PLS_D_32 # ADDI
> This way, we could (and would need to, in fact) keep the 'P' in the prefixed instruction names, but at the same time avoid having extra trans_XX functions just calling another one without any additional code.
I don't understand this at all.
r~
^ permalink raw reply [flat|nested] 63+ messages in thread
* RE: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI
2021-04-30 14:23 ` Richard Henderson
@ 2021-04-30 18:45 ` Luis Fernando Fujita Pires
2021-04-30 19:11 ` Richard Henderson
0 siblings, 1 reply; 63+ messages in thread
From: Luis Fernando Fujita Pires @ 2021-04-30 18:45 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: f4bug, qemu-ppc, lagarcia, Bruno Piazera Larsen,
Matheus Kowalczuk Ferst, david
From: Richard Henderson <richard.henderson@linaro.org>
> On 4/30/21 4:23 AM, Luis Fernando Fujita Pires wrote:
> > I think we should reconsider using the same .decode file for both 32-
> > and 64-bit instructions, to avoid duplicating argument set
> > definitions, and to keep the prefixed instructions close to their non-prefixed
> counterparts.
> varinsnwidth assumes there is no easy way to determine, before decoding, the
> width of the instruction. The way this is implemented in decodetree is vastly less
> optimal than what we can do with a few lines for ppc.
I tried to solve this with one of the previous decodetree patches ("decodetree: Allow custom var width load functions"), whose goal was to allow us to implement a custom instruction load function (in reality, the only effect it had inside decodetree.py was to not generate the _load function).
So the instruction load would still be handled by a simple function inside translate.c, but we would use the auto-generated decode() function to call the trans_XX() functions.
> In addition, there's a rough spot in %field definitions. You can't share those
> between patterns of different sizes, which can get confusing. Have a look at
> target/rx, and the definitions of %b[23]_r_0, which is the same field for 2 and 3-
> byte insns.
Right. In the current patch we're already using separate definitions for 'si' depending on the format (%pls_si and %ds_si below):
&PLS_D rt ra si:int64_t r:bool
%pls_si 32:s18 0:16
@PLS_D ...... .. ... r:1 .. .................. \
...... rt:5 ra:5 ................ \
&PLS_D si=%pls_si
@PLS_D_32 ...... rt:5 ra:5 si:s16 &PLS_D r=0
%ds_si 2:s14 !function=times_4
@PLS_DS_32 ...... rt:5 ra:5 .............. .. &PLS_D si=%ds_si r=0
And I also had to create separate @formats for 32- and 64-bit versions (@PLS_D, @PLS_D_32, etc.), which isn't that nice either.
> The replication of argument set definitions is unfortunate, but in the end will
> only be a handful of lines. We could probably come up with a way to avoid that
> too, via a decodetree extension, if you really insist. (My vague idea there would
> put the argument set definitions into a 3rd file, included on the decodetree
> command-line.)
I think we can already pass multiple files to decodetree.py and it will handle them correctly. I just didn't find a way to do that from the meson build files, which assume decodetree will always use a single input file.
Another option would be to allow files to be included from inside other .decode files.
> > And, in order to share the trans_PADDI/ADDI implementation, maybe add
> something to decodetree.py to allow us to specify that an instruction shares the
> trans_XX() implementation from another one, such as:
> > ADDI 001110 ..... ..... ................ @PLS_D_32 !impl=PADDI
>
> This is done by using the same name up front.
> If you like, add a comment to give the real instruction name.
>
> PADDI 001110 ..... ..... ................ @PLS_D_32 # ADDI
>
>
> > This way, we could (and would need to, in fact) keep the 'P' in the prefixed
> instruction names, but at the same time avoid having extra trans_XX functions
> just calling another one without any additional code.
>
> I don't understand this at all.
Not a big deal. I was just referring to the fact that, in the current patch, you noted that the instruction names in insn64.decode were not prefixed by "P" due to the code sharing with the 32-bit instructions.
Thanks!
Luis
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI
2021-04-30 18:45 ` Luis Fernando Fujita Pires
@ 2021-04-30 19:11 ` Richard Henderson
2021-04-30 20:32 ` Luis Fernando Fujita Pires
0 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 19:11 UTC (permalink / raw)
To: Luis Fernando Fujita Pires, qemu-devel
Cc: f4bug, qemu-ppc, lagarcia, Bruno Piazera Larsen,
Matheus Kowalczuk Ferst, david
On 4/30/21 11:45 AM, Luis Fernando Fujita Pires wrote:
> I think we can already pass multiple files to decodetree.py and it will handle them correctly. I just didn't find a way to do that from the meson build files, which assume decodetree will always use a single input file.
Oh, riscv does this via extra_args:.
r~
^ permalink raw reply [flat|nested] 63+ messages in thread
* RE: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI
2021-04-30 19:11 ` Richard Henderson
@ 2021-04-30 20:32 ` Luis Fernando Fujita Pires
2021-04-30 22:29 ` Richard Henderson
0 siblings, 1 reply; 63+ messages in thread
From: Luis Fernando Fujita Pires @ 2021-04-30 20:32 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: f4bug, qemu-ppc, lagarcia, Bruno Piazera Larsen,
Matheus Kowalczuk Ferst, david
From: Richard Henderson <richard.henderson@linaro.org>
> On 4/30/21 11:45 AM, Luis Fernando Fujita Pires wrote:
> > I think we can already pass multiple files to decodetree.py and it will handle
> them correctly. I just didn't find a way to do that from the meson build files,
> which assume decodetree will always use a single input file.
>
> Oh, riscv does this via extra_args:.
>
> r~
The build system probably will fail to detect that a rebuild is needed if the file passed in through extra_args is changed though, right?
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI
2021-04-30 20:32 ` Luis Fernando Fujita Pires
@ 2021-04-30 22:29 ` Richard Henderson
0 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 22:29 UTC (permalink / raw)
To: Luis Fernando Fujita Pires, qemu-devel
Cc: f4bug, qemu-ppc, lagarcia, Bruno Piazera Larsen,
Matheus Kowalczuk Ferst, david
On 4/30/21 1:32 PM, Luis Fernando Fujita Pires wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
>> On 4/30/21 11:45 AM, Luis Fernando Fujita Pires wrote:
>>> I think we can already pass multiple files to decodetree.py and it will handle
>> them correctly. I just didn't find a way to do that from the meson build files,
>> which assume decodetree will always use a single input file.
>>
>> Oh, riscv does this via extra_args:.
>>
>> r~
>
> The build system probably will fail to detect that a rebuild is needed if the file passed in through extra_args is changed though, right?
Oh, true. Good thing there are patches for riscv to remove that. :-P
r~
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI
2021-04-30 1:15 ` [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI Richard Henderson
2021-04-30 11:23 ` Luis Fernando Fujita Pires
@ 2021-04-30 14:05 ` Matheus K. Ferst
2021-04-30 14:31 ` Richard Henderson
1 sibling, 1 reply; 63+ messages in thread
From: Matheus K. Ferst @ 2021-04-30 14:05 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen, david
On 29/04/2021 22:15, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/ppc/insn32.decode | 12 +++++++
> target/ppc/insn64.decode | 15 +++++++++
> target/ppc/translate.c | 29 ----------------
> target/ppc/translate/fixedpoint-impl.c.inc | 39 ++++++++++++++++++++++
> 4 files changed, 66 insertions(+), 29 deletions(-)
>
> diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
> index b175441209..52d9b355d4 100644
> --- a/target/ppc/insn32.decode
> +++ b/target/ppc/insn32.decode
> @@ -16,3 +16,15 @@
> # You should have received a copy of the GNU Lesser General Public
> # License along with this library; if not, see <http://www.gnu.org/licenses/>.
> #
> +
> +&D rt ra si
> +@D ...... rt:5 ra:5 si:s16 &D
> +
> +# If a prefix is allowed, decode with default values.
> +&PLS_D rt ra si:int64_t r:bool
> +@PLS_D ...... rt:5 ra:5 si:s16 &PLS_D r=0
> +
> +### Fixed-Point Arithmetic Instructions
> +
> +ADDI 001110 ..... ..... ................ @PLS_D
> +ADDIS 001111 ..... ..... ................ @D
> diff --git a/target/ppc/insn64.decode b/target/ppc/insn64.decode
> index 9fc45d0614..f4272df724 100644
> --- a/target/ppc/insn64.decode
> +++ b/target/ppc/insn64.decode
> @@ -16,3 +16,18 @@
> # You should have received a copy of the GNU Lesser General Public
> # License along with this library; if not, see <http://www.gnu.org/licenses/>.
> #
> +
> +# Many all of these instruction names would be prefixed by "P",
> +# but we share code with the non-prefixed instruction.
> +
> +# Format MLS:D and 8LS:D
> +&PLS_D rt ra si:int64_t r:bool !extern
> +%pls_si 32:s18 0:16
> +@PLS_D ...... .. ... r:1 .. .................. \
> + ...... rt:5 ra:5 ................ \
> + &PLS_D si=%pls_si
> +
> +### Fixed-Point Arithmetic Instructions
> +
> +ADDI 000001 10 0--.-- .................. \
> + 001110 ..... ..... ................ @PLS_D
I'm not sure about this. It's a bit surprising to find ADDI here, and
the comment that explains why is likely to be ignored after the big
copyright header.
<snip>
> diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc
> index b740083605..7af1b3bcf5 100644
> --- a/target/ppc/translate/fixedpoint-impl.c.inc
> +++ b/target/ppc/translate/fixedpoint-impl.c.inc
> @@ -16,3 +16,42 @@
> * You should have received a copy of the GNU Lesser General Public
> * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> */
> +
> +/*
> + * Incorporate CIA into the constant when R=1.
> + * Validate that when R=1, RA=0.
> + */
> +static bool resolve_PLS_D(DisasContext *ctx, arg_PLS_D *a)
> +{
> + if (a->r) {
> + if (unlikely(a->ra != 0)) {
> + gen_invalid(ctx);
> + return false;
> + }
> + a->si += ctx->cia;
> + }
> + return true;
> +}
> +
> +static bool trans_ADDI(DisasContext *ctx, arg_PLS_D *a)
> +{
> + if (resolve_PLS_D(ctx, a)) {
> + if (a->ra) {
> + tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_gpr[a->ra], a->si);
> + } else {
> + tcg_gen_movi_tl(cpu_gpr[a->rt], a->si);
> + }
> + }
> + return true;
> +}
> +
I'd prefer to keep a trans_PADDI like
> static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a)
> {
> if(!resolve_PLS_D(ctx, a)) {
> return false;
> }
> return trans_ADDI(ctx, a);
> }
It's the middle way between v2 and v3. trans_ADDI code is reused, it'll
probably be optimized as a tail call, and resolve_PLS_D is not called
when it's not needed.
> +static bool trans_ADDIS(DisasContext *ctx, arg_D *a)
> +{
> + int si = a->si << 16;
> + if (a->ra) {
> + tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_gpr[a->ra], si);
> + } else {
> + tcg_gen_movi_tl(cpu_gpr[a->rt], si);
> + }
> + return true;
> +}
>
I'd also keep this as in the last version, where trans_ADDI is called.
Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software Júnior
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI
2021-04-30 14:05 ` Matheus K. Ferst
@ 2021-04-30 14:31 ` Richard Henderson
2021-04-30 18:02 ` Matheus K. Ferst
0 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 14:31 UTC (permalink / raw)
To: Matheus K. Ferst, qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen, david
On 4/30/21 7:05 AM, Matheus K. Ferst wrote:
>> +ADDI 000001 10 0--.-- .................. \
>> + 001110 ..... ..... ................ @PLS_D
>
> I'm not sure about this. It's a bit surprising to find ADDI here, and the
> comment that explains why is likely to be ignored after the big copyright header.
You could move the comment closer, and replicate, e.g.
ADDI .... \
.... @PLS_D # PADDI
> I'd prefer to keep a trans_PADDI like
>
> > static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a)
> > {
> > if(!resolve_PLS_D(ctx, a)) {
> > return false;
> > }
> > return trans_ADDI(ctx, a);
> > }
But in this case ADDI probably doesn't use PLS_D. You could use
static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a)
{
arg_D d;
if (!resolve_PLS_D(ctx, &d, a)) {
return false;
}
return trans_ADDI(ctx, &d);
}
making sure to use int64_t in the offset for arg_D.
> It's the middle way between v2 and v3. trans_ADDI code is reused, it'll
> probably be optimized as a tail call, and resolve_PLS_D is not called when it's
> not needed.
My version won't tail-call, because of the escaping local storage, but I don't
see how you can avoid it.
r~
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI
2021-04-30 14:31 ` Richard Henderson
@ 2021-04-30 18:02 ` Matheus K. Ferst
2021-04-30 18:43 ` Richard Henderson
0 siblings, 1 reply; 63+ messages in thread
From: Matheus K. Ferst @ 2021-04-30 18:02 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen, david
On 30/04/2021 11:31, Richard Henderson wrote:
> On 4/30/21 7:05 AM, Matheus K. Ferst wrote:
>>> +ADDI 000001 10 0--.-- .................. \
>>> + 001110 ..... ..... ................ @PLS_D
>>
>> I'm not sure about this. It's a bit surprising to find ADDI here, and
>> the comment that explains why is likely to be ignored after the big
>> copyright header.
>
> You could move the comment closer, and replicate, e.g.
>
> ADDI .... \
> .... @PLS_D # PADDI
>
>
If we keep this naming, IMHO moving the comment closer looks better.
>> I'd prefer to keep a trans_PADDI like
>>
>> > static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a)
>> > {
>> > if(!resolve_PLS_D(ctx, a)) {
>> > return false;
>> > }
>> > return trans_ADDI(ctx, a);
>> > }
>
> But in this case ADDI probably doesn't use PLS_D. You could use
>
> static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a)
> {
> arg_D d;
> if (!resolve_PLS_D(ctx, &d, a)) {
> return false;
> }
> return trans_ADDI(ctx, &d);
> }
>
> making sure to use int64_t in the offset for arg_D.
>
We'd keep trans_ADDI with the same signature to avoid creating an arg_D
on the stack. Patch 4 added type specification, maybe we can define an
arg_D within arg_PLD_D? I'll play a bit to see if it works.
>> It's the middle way between v2 and v3. trans_ADDI code is reused,
>> it'll probably be optimized as a tail call, and resolve_PLS_D is not
>> called when it's not needed.
>
> My version won't tail-call, because of the escaping local storage, but I
> don't see how you can avoid it.
>
>
> r~
I haven't been able to test it properly yet, but at least on godbolt it
seems that the compiler prefers to inline over tail call, so maybe
that's not a problem.
Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software Júnior
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI
2021-04-30 18:02 ` Matheus K. Ferst
@ 2021-04-30 18:43 ` Richard Henderson
2021-04-30 23:29 ` Matheus K. Ferst
0 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 18:43 UTC (permalink / raw)
To: Matheus K. Ferst, qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen, david
On 4/30/21 11:02 AM, Matheus K. Ferst wrote:
>> But in this case ADDI probably doesn't use PLS_D. You could use
>>
>> static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a)
>> {
>> arg_D d;
>> if (!resolve_PLS_D(ctx, &d, a)) {
>> return false;
>> }
>> return trans_ADDI(ctx, &d);
>> }
>>
>> making sure to use int64_t in the offset for arg_D.
>>
>
> We'd keep trans_ADDI with the same signature to avoid creating an arg_D on the
> stack. Patch 4 added type specification, maybe we can define an arg_D within
> arg_PLD_D? I'll play a bit to see if it works.
That starts to creep, with e.g. ADDIS now requiring arg_PLD_D. You'll want to
audit the other D-form insns to see what other special cases there are.
r~
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI
2021-04-30 18:43 ` Richard Henderson
@ 2021-04-30 23:29 ` Matheus K. Ferst
0 siblings, 0 replies; 63+ messages in thread
From: Matheus K. Ferst @ 2021-04-30 23:29 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen, david
On 30/04/2021 15:43, Richard Henderson wrote:
> On 4/30/21 11:02 AM, Matheus K. Ferst wrote:
>>> But in this case ADDI probably doesn't use PLS_D. You could use
>>>
>>> static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a) { arg_D
>>> d; if (!resolve_PLS_D(ctx, &d, a)) { return false; } return
>>> trans_ADDI(ctx, &d); }
>>>
>>> making sure to use int64_t in the offset for arg_D.
>>>
>>
>> We'd keep trans_ADDI with the same signature to avoid creating an
>> arg_D on the stack. Patch 4 added type specification, maybe we can
>> define an arg_D within arg_PLD_D? I'll play a bit to see if it
>> works.
>
> That starts to creep, with e.g. ADDIS now requiring arg_PLD_D. You'll
> want to audit the other D-form insns to see what other special cases
> there are.
>
> r~
Well, anything that shares implementation with a prefixed instruction
would use the prefixed arg.
I got arg_D within arg_PLD_D using !function and calling
decode_insn32_extract_D. A bit on the ugly side, and probably not
worth the effort. Maybe changing decodetree would help, but that's
another patch, for another series.
Anyway, my compiler (GCC 9.3) is inlining trans_ADDI calls with -O3, so
I'd say that your trans_PADDI from the previous message, with trans_ADDI
and trans_ADDIS using arg_D, would be the cleaner solution.
Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software Júnior
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH v3 26/30] target/ppc: Implement PNOP
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (24 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-05-03 22:41 ` Philippe Mathieu-Daudé
2021-04-30 1:15 ` [PATCH v3 27/30] target/ppc: Move D/DS/X-form integer loads to decodetree Richard Henderson
` (4 subsequent siblings)
30 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/insn32.decode | 2 ++
target/ppc/insn64.decode | 11 +++++++++++
target/ppc/translate/fixedpoint-impl.c.inc | 5 +++++
3 files changed, 18 insertions(+)
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index 52d9b355d4..2ed25c7e67 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -17,6 +17,8 @@
# License along with this library; if not, see <http://www.gnu.org/licenses/>.
#
+&empty
+
&D rt ra si
@D ...... rt:5 ra:5 si:s16 &D
diff --git a/target/ppc/insn64.decode b/target/ppc/insn64.decode
index f4272df724..5a82ce375e 100644
--- a/target/ppc/insn64.decode
+++ b/target/ppc/insn64.decode
@@ -20,6 +20,8 @@
# Many all of these instruction names would be prefixed by "P",
# but we share code with the non-prefixed instruction.
+&empty !extern
+
# Format MLS:D and 8LS:D
&PLS_D rt ra si:int64_t r:bool !extern
%pls_si 32:s18 0:16
@@ -31,3 +33,12 @@
ADDI 000001 10 0--.-- .................. \
001110 ..... ..... ................ @PLS_D
+
+### Prefixed No-operation Instruction
+
+# TODO: diagnose the set of patterns that are illegal:
+# branches, rfebb, sync other than isync, or a service processor attention.
+# The Engineering Note allows us to either diagnose these as illegal,
+# or treat them all as no-op.
+NOP 000001 11 0000-- 000000000000000000 \
+ --------------------------------
diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc
index 7af1b3bcf5..96b8c38f60 100644
--- a/target/ppc/translate/fixedpoint-impl.c.inc
+++ b/target/ppc/translate/fixedpoint-impl.c.inc
@@ -55,3 +55,8 @@ static bool trans_ADDIS(DisasContext *ctx, arg_D *a)
}
return true;
}
+
+static bool trans_NOP(DisasContext *ctx, arg_NOP *a)
+{
+ return true;
+}
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH v3 26/30] target/ppc: Implement PNOP
2021-04-30 1:15 ` [PATCH v3 26/30] target/ppc: Implement PNOP Richard Henderson
@ 2021-05-03 22:41 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 63+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-03 22:41 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: luis.pires, qemu-ppc, lagarcia, bruno.larsen, matheus.ferst, david
On 4/30/21 3:15 AM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/ppc/insn32.decode | 2 ++
> target/ppc/insn64.decode | 11 +++++++++++
> target/ppc/translate/fixedpoint-impl.c.inc | 5 +++++
> 3 files changed, 18 insertions(+)
>
> diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
> index 52d9b355d4..2ed25c7e67 100644
> --- a/target/ppc/insn32.decode
> +++ b/target/ppc/insn32.decode
> @@ -17,6 +17,8 @@
> # License along with this library; if not, see <http://www.gnu.org/licenses/>.
> #
>
> +&empty
> +
> +static bool trans_NOP(DisasContext *ctx, arg_NOP *a)
Matter of taste, I'd rather use 'arg_empty'.
> +{
> + return true;
> +}
>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH v3 27/30] target/ppc: Move D/DS/X-form integer loads to decodetree
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (25 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 26/30] target/ppc: Implement PNOP Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 23:54 ` Matheus K. Ferst
2021-04-30 1:15 ` [PATCH v3 28/30] target/ppc: Implement prefixed integer load instructions Richard Henderson
` (3 subsequent siblings)
30 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
These are all connected by macros in the legacy decoding.
Decode the D and DS forms into the PLS_D argument set so
that prefixed insns can share code.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/insn32.decode | 37 ++++++
target/ppc/translate.c | 145 ++++-----------------
target/ppc/translate/fixedpoint-impl.c.inc | 114 ++++++++++++++++
3 files changed, 174 insertions(+), 122 deletions(-)
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index 2ed25c7e67..1c1b4620fc 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -26,6 +26,43 @@
&PLS_D rt ra si:int64_t r:bool
@PLS_D ...... rt:5 ra:5 si:s16 &PLS_D r=0
+%ds_si 2:s14 !function=times_4
+@PLS_DS ...... rt:5 ra:5 .............. .. &PLS_D si=%ds_si r=0
+
+&X rt ra rb
+@X ...... rt:5 ra:5 rb:5 .......... . &X
+
+### Fixed-Point Load Instructions
+
+LBZ 100010 ..... ..... ................ @PLS_D
+LBZU 100011 ..... ..... ................ @PLS_D
+LBZX 011111 ..... ..... ..... 0001010111 - @X
+LBZUX 011111 ..... ..... ..... 0001110111 - @X
+
+LHZ 101000 ..... ..... ................ @PLS_D
+LHZU 101001 ..... ..... ................ @PLS_D
+LHZX 011111 ..... ..... ..... 0100010111 - @X
+LHZUX 011111 ..... ..... ..... 0100110111 - @X
+
+LHA 101010 ..... ..... ................ @PLS_D
+LHAU 101011 ..... ..... ................ @PLS_D
+LHAX 011111 ..... ..... ..... 0101010111 - @X
+LHAXU 011111 ..... ..... ..... 0101110111 - @X
+
+LWZ 100000 ..... ..... ................ @PLS_D
+LWZU 100001 ..... ..... ................ @PLS_D
+LWZX 011111 ..... ..... ..... 0000010111 - @X
+LWZUX 011111 ..... ..... ..... 0000110111 - @X
+
+LWA 111010 ..... ..... ..............10 @PLS_DS
+LWAX 011111 ..... ..... ..... 0101010101 - @X
+LWAUX 011111 ..... ..... ..... 0101110101 - @X
+
+LD 111010 ..... ..... ..............00 @PLS_DS
+LDU 111010 ..... ..... ..............01 @PLS_DS
+LDX 011111 ..... ..... ..... 0000010101 - @X
+LDUX 011111 ..... ..... ..... 0000110101 - @X
+
### Fixed-Point Arithmetic Instructions
ADDI 001110 ..... ..... ................ @PLS_D
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 5a8a3c39c3..1fdb501ee9 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -2487,54 +2487,6 @@ GEN_QEMU_STORE_64(st64, DEF_MEMOP(MO_Q))
GEN_QEMU_STORE_64(st64r, BSWAP_MEMOP(MO_Q))
#endif
-#define GEN_LD(name, ldop, opc, type) \
-static void glue(gen_, name)(DisasContext *ctx) \
-{ \
- TCGv EA; \
- gen_set_access_type(ctx, ACCESS_INT); \
- EA = tcg_temp_new(); \
- gen_addr_imm_index(ctx, EA, 0); \
- gen_qemu_##ldop(ctx, cpu_gpr[rD(ctx->opcode)], EA); \
- tcg_temp_free(EA); \
-}
-
-#define GEN_LDU(name, ldop, opc, type) \
-static void glue(gen_, name##u)(DisasContext *ctx) \
-{ \
- TCGv EA; \
- if (unlikely(rA(ctx->opcode) == 0 || \
- rA(ctx->opcode) == rD(ctx->opcode))) { \
- gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL); \
- return; \
- } \
- gen_set_access_type(ctx, ACCESS_INT); \
- EA = tcg_temp_new(); \
- if (type == PPC_64B) \
- gen_addr_imm_index(ctx, EA, 0x03); \
- else \
- gen_addr_imm_index(ctx, EA, 0); \
- gen_qemu_##ldop(ctx, cpu_gpr[rD(ctx->opcode)], EA); \
- tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], EA); \
- tcg_temp_free(EA); \
-}
-
-#define GEN_LDUX(name, ldop, opc2, opc3, type) \
-static void glue(gen_, name##ux)(DisasContext *ctx) \
-{ \
- TCGv EA; \
- if (unlikely(rA(ctx->opcode) == 0 || \
- rA(ctx->opcode) == rD(ctx->opcode))) { \
- gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL); \
- return; \
- } \
- gen_set_access_type(ctx, ACCESS_INT); \
- EA = tcg_temp_new(); \
- gen_addr_reg_index(ctx, EA); \
- gen_qemu_##ldop(ctx, cpu_gpr[rD(ctx->opcode)], EA); \
- tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], EA); \
- tcg_temp_free(EA); \
-}
-
#define GEN_LDX_E(name, ldop, opc2, opc3, type, type2, chk) \
static void glue(gen_, name##x)(DisasContext *ctx) \
{ \
@@ -2553,21 +2505,6 @@ static void glue(gen_, name##x)(DisasContext *ctx) \
#define GEN_LDX_HVRM(name, ldop, opc2, opc3, type) \
GEN_LDX_E(name, ldop, opc2, opc3, type, PPC_NONE, CHK_HVRM)
-#define GEN_LDS(name, ldop, op, type) \
-GEN_LD(name, ldop, op | 0x20, type); \
-GEN_LDU(name, ldop, op | 0x21, type); \
-GEN_LDUX(name, ldop, 0x17, op | 0x01, type); \
-GEN_LDX(name, ldop, 0x17, op | 0x00, type)
-
-/* lbz lbzu lbzux lbzx */
-GEN_LDS(lbz, ld8u, 0x02, PPC_INTEGER);
-/* lha lhau lhaux lhax */
-GEN_LDS(lha, ld16s, 0x0A, PPC_INTEGER);
-/* lhz lhzu lhzux lhzx */
-GEN_LDS(lhz, ld16u, 0x08, PPC_INTEGER);
-/* lwz lwzu lwzux lwzx */
-GEN_LDS(lwz, ld32u, 0x00, PPC_INTEGER);
-
#define GEN_LDEPX(name, ldop, opc2, opc3) \
static void glue(gen_, name##epx)(DisasContext *ctx) \
{ \
@@ -2588,47 +2525,12 @@ GEN_LDEPX(ld, DEF_MEMOP(MO_Q), 0x1D, 0x00)
#endif
#if defined(TARGET_PPC64)
-/* lwaux */
-GEN_LDUX(lwa, ld32s, 0x15, 0x0B, PPC_64B);
-/* lwax */
-GEN_LDX(lwa, ld32s, 0x15, 0x0A, PPC_64B);
-/* ldux */
-GEN_LDUX(ld, ld64_i64, 0x15, 0x01, PPC_64B);
-/* ldx */
-GEN_LDX(ld, ld64_i64, 0x15, 0x00, PPC_64B);
-
/* CI load/store variants */
GEN_LDX_HVRM(ldcix, ld64_i64, 0x15, 0x1b, PPC_CILDST)
GEN_LDX_HVRM(lwzcix, ld32u, 0x15, 0x15, PPC_CILDST)
GEN_LDX_HVRM(lhzcix, ld16u, 0x15, 0x19, PPC_CILDST)
GEN_LDX_HVRM(lbzcix, ld8u, 0x15, 0x1a, PPC_CILDST)
-static void gen_ld(DisasContext *ctx)
-{
- TCGv EA;
- if (Rc(ctx->opcode)) {
- if (unlikely(rA(ctx->opcode) == 0 ||
- rA(ctx->opcode) == rD(ctx->opcode))) {
- gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
- return;
- }
- }
- gen_set_access_type(ctx, ACCESS_INT);
- EA = tcg_temp_new();
- gen_addr_imm_index(ctx, EA, 0x03);
- if (ctx->opcode & 0x02) {
- /* lwa (lwau is undefined) */
- gen_qemu_ld32s(ctx, cpu_gpr[rD(ctx->opcode)], EA);
- } else {
- /* ld - ldu */
- gen_qemu_ld64_i64(ctx, cpu_gpr[rD(ctx->opcode)], EA);
- }
- if (Rc(ctx->opcode)) {
- tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], EA);
- }
- tcg_temp_free(EA);
-}
-
/* lq */
static void gen_lq(DisasContext *ctx)
{
@@ -6849,6 +6751,14 @@ static inline void set_avr64(int regno, TCGv_i64 src, bool high)
tcg_gen_st_i64(src, cpu_env, avr64_offset(regno, high));
}
+/*
+ * Helpers for decodetree used by !function for decoding arguments.
+ */
+static int times_4(DisasContext *ctx, int x)
+{
+ return x * 4;
+}
+
/*
* Helpers for trans_* functions to check for specific insns flags.
* Use token pasting to ensure that we use the proper flag with the
@@ -6875,6 +6785,21 @@ static inline void set_avr64(int regno, TCGv_i64 src, bool high)
# define REQUIRE_64BIT(CTX) REQUIRE_INSNS_FLAGS(CTX, 64B)
#endif
+/*
+ * Helpers for implementing sets of trans_* functions.
+ * Defer the implementation of NAME to FUNC, with optional extra arguments.
+ */
+#define TRANS(NAME, FUNC, ...) \
+ static bool trans_##NAME(DisasContext *ctx, arg_##NAME *a) \
+ { return FUNC(ctx, a, __VA_ARGS__); }
+
+#define TRANS64(NAME, FUNC, ...) \
+ static bool trans_##NAME(DisasContext *ctx, arg_##NAME *a) \
+ { REQUIRE_64BIT(ctx); return FUNC(ctx, a, __VA_ARGS__); }
+
+/* TODO: More TRANS* helpers for extra insn_flags checks. */
+
+
#include "decode-insn32.c.inc"
#include "decode-insn64.c.inc"
#include "translate/fixedpoint-impl.c.inc"
@@ -7059,7 +6984,6 @@ GEN_HANDLER2_E(extswsli1, "extswsli", 0x1F, 0x1B, 0x1B, 0x00000000,
PPC_NONE, PPC2_ISA300),
#endif
#if defined(TARGET_PPC64)
-GEN_HANDLER(ld, 0x3A, 0xFF, 0xFF, 0x00000000, PPC_64B),
GEN_HANDLER(lq, 0x38, 0xFF, 0xFF, 0x00000000, PPC_64BX),
GEN_HANDLER(std, 0x3E, 0xFF, 0xFF, 0x00000000, PPC_64B),
#endif
@@ -7425,34 +7349,11 @@ GEN_PPC64_R2(rldcr, 0x1E, 0x09),
GEN_PPC64_R4(rldimi, 0x1E, 0x06),
#endif
-#undef GEN_LD
-#undef GEN_LDU
-#undef GEN_LDUX
#undef GEN_LDX_E
-#undef GEN_LDS
-#define GEN_LD(name, ldop, opc, type) \
-GEN_HANDLER(name, opc, 0xFF, 0xFF, 0x00000000, type),
-#define GEN_LDU(name, ldop, opc, type) \
-GEN_HANDLER(name##u, opc, 0xFF, 0xFF, 0x00000000, type),
-#define GEN_LDUX(name, ldop, opc2, opc3, type) \
-GEN_HANDLER(name##ux, 0x1F, opc2, opc3, 0x00000001, type),
#define GEN_LDX_E(name, ldop, opc2, opc3, type, type2, chk) \
GEN_HANDLER_E(name##x, 0x1F, opc2, opc3, 0x00000001, type, type2),
-#define GEN_LDS(name, ldop, op, type) \
-GEN_LD(name, ldop, op | 0x20, type) \
-GEN_LDU(name, ldop, op | 0x21, type) \
-GEN_LDUX(name, ldop, 0x17, op | 0x01, type) \
-GEN_LDX(name, ldop, 0x17, op | 0x00, type)
-GEN_LDS(lbz, ld8u, 0x02, PPC_INTEGER)
-GEN_LDS(lha, ld16s, 0x0A, PPC_INTEGER)
-GEN_LDS(lhz, ld16u, 0x08, PPC_INTEGER)
-GEN_LDS(lwz, ld32u, 0x00, PPC_INTEGER)
#if defined(TARGET_PPC64)
-GEN_LDUX(lwa, ld32s, 0x15, 0x0B, PPC_64B)
-GEN_LDX(lwa, ld32s, 0x15, 0x0A, PPC_64B)
-GEN_LDUX(ld, ld64_i64, 0x15, 0x01, PPC_64B)
-GEN_LDX(ld, ld64_i64, 0x15, 0x00, PPC_64B)
GEN_LDX_E(ldbr, ld64ur_i64, 0x14, 0x10, PPC_NONE, PPC2_DBRX, CHK_NONE)
/* HV/P7 and later only */
diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc
index 96b8c38f60..cb3219c996 100644
--- a/target/ppc/translate/fixedpoint-impl.c.inc
+++ b/target/ppc/translate/fixedpoint-impl.c.inc
@@ -33,6 +33,120 @@ static bool resolve_PLS_D(DisasContext *ctx, arg_PLS_D *a)
return true;
}
+/*
+ * Fixed-Point Load/Store Instructions
+ */
+
+static bool do_ldst_PLS_D(DisasContext *ctx, arg_PLS_D *a, bool update,
+ bool store, MemOp mop)
+{
+ TCGv ea;
+
+ if (!resolve_PLS_D(ctx, a)) {
+ return true;
+ }
+ if (update && (a->ra == 0 || (!store && a->ra == a->rt))) {
+ gen_invalid(ctx);
+ return true;
+ }
+ gen_set_access_type(ctx, ACCESS_INT);
+
+ ea = tcg_temp_new();
+ if (a->ra) {
+ tcg_gen_addi_tl(ea, cpu_gpr[a->ra], a->si);
+ } else {
+ tcg_gen_movi_tl(ea, a->si);
+ }
+ if (NARROW_MODE(ctx)) {
+ tcg_gen_ext32u_tl(ea, ea);
+ }
+ mop ^= ctx->default_tcg_memop_mask;
+ if (store) {
+ tcg_gen_qemu_st_tl(cpu_gpr[a->rt], ea, ctx->mem_idx, mop);
+ } else {
+ tcg_gen_qemu_ld_tl(cpu_gpr[a->rt], ea, ctx->mem_idx, mop);
+ }
+ if (update) {
+ tcg_gen_mov_tl(cpu_gpr[a->ra], ea);
+ }
+ tcg_temp_free(ea);
+
+ return true;
+}
+
+static bool do_ldst_X(DisasContext *ctx, arg_X *a, bool update,
+ bool store, MemOp mop)
+{
+ TCGv ea;
+
+ if (update && (a->ra == 0 || (!store && a->ra == a->rt))) {
+ gen_invalid(ctx);
+ return true;
+ }
+ gen_set_access_type(ctx, ACCESS_INT);
+
+ ea = tcg_temp_new();
+ if (a->ra) {
+ tcg_gen_add_tl(ea, cpu_gpr[a->ra], cpu_gpr[a->rb]);
+ } else {
+ tcg_gen_mov_tl(ea, cpu_gpr[a->rb]);
+ }
+ if (NARROW_MODE(ctx)) {
+ tcg_gen_ext32u_tl(ea, ea);
+ }
+ mop ^= ctx->default_tcg_memop_mask;
+ if (store) {
+ tcg_gen_qemu_st_tl(cpu_gpr[a->rt], ea, ctx->mem_idx, mop);
+ } else {
+ tcg_gen_qemu_ld_tl(cpu_gpr[a->rt], ea, ctx->mem_idx, mop);
+ }
+ if (update) {
+ tcg_gen_mov_tl(cpu_gpr[a->ra], ea);
+ }
+ tcg_temp_free(ea);
+
+ return true;
+}
+
+/* Load Byte and Zero */
+TRANS(LBZ, do_ldst_PLS_D, false, false, MO_UB)
+TRANS(LBZX, do_ldst_X, false, false, MO_UB)
+TRANS(LBZU, do_ldst_PLS_D, true, false, MO_UB)
+TRANS(LBZUX, do_ldst_X, true, false, MO_UB)
+
+/* Load Halfword and Zero */
+TRANS(LHZ, do_ldst_PLS_D, false, false, MO_UW)
+TRANS(LHZX, do_ldst_X, false, false, MO_UW)
+TRANS(LHZU, do_ldst_PLS_D, true, false, MO_UW)
+TRANS(LHZUX, do_ldst_X, true, false, MO_UW)
+
+/* Load Halfword Algebraic */
+TRANS(LHA, do_ldst_PLS_D, false, false, MO_SW)
+TRANS(LHAX, do_ldst_X, false, false, MO_SW)
+TRANS(LHAU, do_ldst_PLS_D, true, false, MO_SW)
+TRANS(LHAXU, do_ldst_X, true, false, MO_SW)
+
+/* Load Word and Zero */
+TRANS(LWZ, do_ldst_PLS_D, false, false, MO_UL)
+TRANS(LWZX, do_ldst_X, false, false, MO_UL)
+TRANS(LWZU, do_ldst_PLS_D, true, false, MO_UL)
+TRANS(LWZUX, do_ldst_X, true, false, MO_UL)
+
+/* Load Word Algebraic */
+TRANS64(LWA, do_ldst_PLS_D, false, false, MO_SL)
+TRANS64(LWAX, do_ldst_X, false, false, MO_SL)
+TRANS64(LWAUX, do_ldst_X, true, false, MO_SL)
+
+/* Load Doubleword */
+TRANS64(LD, do_ldst_PLS_D, false, false, MO_Q)
+TRANS64(LDX, do_ldst_X, false, false, MO_Q)
+TRANS64(LDU, do_ldst_PLS_D, true, false, MO_Q)
+TRANS64(LDUX, do_ldst_X, true, false, MO_Q)
+
+/*
+ * Fixed-Point Arithmetic Instructions
+ */
+
static bool trans_ADDI(DisasContext *ctx, arg_PLS_D *a)
{
if (resolve_PLS_D(ctx, a)) {
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH v3 27/30] target/ppc: Move D/DS/X-form integer loads to decodetree
2021-04-30 1:15 ` [PATCH v3 27/30] target/ppc: Move D/DS/X-form integer loads to decodetree Richard Henderson
@ 2021-04-30 23:54 ` Matheus K. Ferst
2021-05-01 0:50 ` Richard Henderson
0 siblings, 1 reply; 63+ messages in thread
From: Matheus K. Ferst @ 2021-04-30 23:54 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen, david
On 29/04/2021 22:15, Richard Henderson wrote:
> These are all connected by macros in the legacy decoding.
> Decode the D and DS forms into the PLS_D argument set so
> that prefixed insns can share code.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/ppc/insn32.decode | 37 ++++++
> target/ppc/translate.c | 145 ++++-----------------
> target/ppc/translate/fixedpoint-impl.c.inc | 114 ++++++++++++++++
> 3 files changed, 174 insertions(+), 122 deletions(-)
>
> diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
> index 2ed25c7e67..1c1b4620fc 100644
> --- a/target/ppc/insn32.decode
> +++ b/target/ppc/insn32.decode
> @@ -26,6 +26,43 @@
> &PLS_D rt ra si:int64_t r:bool
> @PLS_D ...... rt:5 ra:5 si:s16 &PLS_D r=0
>
> +%ds_si 2:s14 !function=times_4
> +@PLS_DS ...... rt:5 ra:5 .............. .. &PLS_D si=%ds_si r=0
> +
> +&X rt ra rb
> +@X ...... rt:5 ra:5 rb:5 .......... . &X
> +
> +### Fixed-Point Load Instructions
> +
> +LBZ 100010 ..... ..... ................ @PLS_D
> +LBZU 100011 ..... ..... ................ @PLS_D
> +LBZX 011111 ..... ..... ..... 0001010111 - @X
> +LBZUX 011111 ..... ..... ..... 0001110111 - @X
> +
> +LHZ 101000 ..... ..... ................ @PLS_D
> +LHZU 101001 ..... ..... ................ @PLS_D
> +LHZX 011111 ..... ..... ..... 0100010111 - @X
> +LHZUX 011111 ..... ..... ..... 0100110111 - @X
> +
> +LHA 101010 ..... ..... ................ @PLS_D
> +LHAU 101011 ..... ..... ................ @PLS_D
> +LHAX 011111 ..... ..... ..... 0101010111 - @X
> +LHAXU 011111 ..... ..... ..... 0101110111 - @X
> +
> +LWZ 100000 ..... ..... ................ @PLS_D
> +LWZU 100001 ..... ..... ................ @PLS_D
> +LWZX 011111 ..... ..... ..... 0000010111 - @X
> +LWZUX 011111 ..... ..... ..... 0000110111 - @X
> +
> +LWA 111010 ..... ..... ..............10 @PLS_DS
> +LWAX 011111 ..... ..... ..... 0101010101 - @X
> +LWAUX 011111 ..... ..... ..... 0101110101 - @X
> +
> +LD 111010 ..... ..... ..............00 @PLS_DS
> +LDU 111010 ..... ..... ..............01 @PLS_DS
> +LDX 011111 ..... ..... ..... 0000010101 - @X
> +LDUX 011111 ..... ..... ..... 0000110101 - @X
> +
> ### Fixed-Point Arithmetic Instructions
>
> ADDI 001110 ..... ..... ................ @PLS_D
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 5a8a3c39c3..1fdb501ee9 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -2487,54 +2487,6 @@ GEN_QEMU_STORE_64(st64, DEF_MEMOP(MO_Q))
> GEN_QEMU_STORE_64(st64r, BSWAP_MEMOP(MO_Q))
> #endif
>
> -#define GEN_LD(name, ldop, opc, type) \
> -static void glue(gen_, name)(DisasContext *ctx) \
> -{ \
> - TCGv EA; \
> - gen_set_access_type(ctx, ACCESS_INT); \
> - EA = tcg_temp_new(); \
> - gen_addr_imm_index(ctx, EA, 0); \
> - gen_qemu_##ldop(ctx, cpu_gpr[rD(ctx->opcode)], EA); \
> - tcg_temp_free(EA); \
> -}
> -
> -#define GEN_LDU(name, ldop, opc, type) \
> -static void glue(gen_, name##u)(DisasContext *ctx) \
> -{ \
> - TCGv EA; \
> - if (unlikely(rA(ctx->opcode) == 0 || \
> - rA(ctx->opcode) == rD(ctx->opcode))) { \
> - gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL); \
> - return; \
> - } \
> - gen_set_access_type(ctx, ACCESS_INT); \
> - EA = tcg_temp_new(); \
> - if (type == PPC_64B) \
> - gen_addr_imm_index(ctx, EA, 0x03); \
> - else \
> - gen_addr_imm_index(ctx, EA, 0); \
> - gen_qemu_##ldop(ctx, cpu_gpr[rD(ctx->opcode)], EA); \
> - tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], EA); \
> - tcg_temp_free(EA); \
> -}
> -
> -#define GEN_LDUX(name, ldop, opc2, opc3, type) \
> -static void glue(gen_, name##ux)(DisasContext *ctx) \
> -{ \
> - TCGv EA; \
> - if (unlikely(rA(ctx->opcode) == 0 || \
> - rA(ctx->opcode) == rD(ctx->opcode))) { \
> - gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL); \
> - return; \
> - } \
> - gen_set_access_type(ctx, ACCESS_INT); \
> - EA = tcg_temp_new(); \
> - gen_addr_reg_index(ctx, EA); \
> - gen_qemu_##ldop(ctx, cpu_gpr[rD(ctx->opcode)], EA); \
> - tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], EA); \
> - tcg_temp_free(EA); \
> -}
> -
> #define GEN_LDX_E(name, ldop, opc2, opc3, type, type2, chk) \
> static void glue(gen_, name##x)(DisasContext *ctx) \
> { \
> @@ -2553,21 +2505,6 @@ static void glue(gen_, name##x)(DisasContext *ctx) \
> #define GEN_LDX_HVRM(name, ldop, opc2, opc3, type) \
> GEN_LDX_E(name, ldop, opc2, opc3, type, PPC_NONE, CHK_HVRM)
>
> -#define GEN_LDS(name, ldop, op, type) \
> -GEN_LD(name, ldop, op | 0x20, type); \
> -GEN_LDU(name, ldop, op | 0x21, type); \
> -GEN_LDUX(name, ldop, 0x17, op | 0x01, type); \
> -GEN_LDX(name, ldop, 0x17, op | 0x00, type)
> -
> -/* lbz lbzu lbzux lbzx */
> -GEN_LDS(lbz, ld8u, 0x02, PPC_INTEGER);
> -/* lha lhau lhaux lhax */
> -GEN_LDS(lha, ld16s, 0x0A, PPC_INTEGER);
> -/* lhz lhzu lhzux lhzx */
> -GEN_LDS(lhz, ld16u, 0x08, PPC_INTEGER);
> -/* lwz lwzu lwzux lwzx */
> -GEN_LDS(lwz, ld32u, 0x00, PPC_INTEGER);
> -
> #define GEN_LDEPX(name, ldop, opc2, opc3) \
> static void glue(gen_, name##epx)(DisasContext *ctx) \
> { \
> @@ -2588,47 +2525,12 @@ GEN_LDEPX(ld, DEF_MEMOP(MO_Q), 0x1D, 0x00)
> #endif
>
> #if defined(TARGET_PPC64)
> -/* lwaux */
> -GEN_LDUX(lwa, ld32s, 0x15, 0x0B, PPC_64B);
> -/* lwax */
> -GEN_LDX(lwa, ld32s, 0x15, 0x0A, PPC_64B);
> -/* ldux */
> -GEN_LDUX(ld, ld64_i64, 0x15, 0x01, PPC_64B);
> -/* ldx */
> -GEN_LDX(ld, ld64_i64, 0x15, 0x00, PPC_64B);
> -
> /* CI load/store variants */
> GEN_LDX_HVRM(ldcix, ld64_i64, 0x15, 0x1b, PPC_CILDST)
> GEN_LDX_HVRM(lwzcix, ld32u, 0x15, 0x15, PPC_CILDST)
> GEN_LDX_HVRM(lhzcix, ld16u, 0x15, 0x19, PPC_CILDST)
> GEN_LDX_HVRM(lbzcix, ld8u, 0x15, 0x1a, PPC_CILDST)
>
> -static void gen_ld(DisasContext *ctx)
> -{
> - TCGv EA;
> - if (Rc(ctx->opcode)) {
> - if (unlikely(rA(ctx->opcode) == 0 ||
> - rA(ctx->opcode) == rD(ctx->opcode))) {
> - gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
> - return;
> - }
> - }
> - gen_set_access_type(ctx, ACCESS_INT);
> - EA = tcg_temp_new();
> - gen_addr_imm_index(ctx, EA, 0x03);
> - if (ctx->opcode & 0x02) {
> - /* lwa (lwau is undefined) */
> - gen_qemu_ld32s(ctx, cpu_gpr[rD(ctx->opcode)], EA);
> - } else {
> - /* ld - ldu */
> - gen_qemu_ld64_i64(ctx, cpu_gpr[rD(ctx->opcode)], EA);
> - }
> - if (Rc(ctx->opcode)) {
> - tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], EA);
> - }
> - tcg_temp_free(EA);
> -}
> -
> /* lq */
> static void gen_lq(DisasContext *ctx)
> {
> @@ -6849,6 +6751,14 @@ static inline void set_avr64(int regno, TCGv_i64 src, bool high)
> tcg_gen_st_i64(src, cpu_env, avr64_offset(regno, high));
> }
>
> +/*
> + * Helpers for decodetree used by !function for decoding arguments.
> + */
> +static int times_4(DisasContext *ctx, int x)
> +{
> + return x * 4;
> +}
> +
> /*
> * Helpers for trans_* functions to check for specific insns flags.
> * Use token pasting to ensure that we use the proper flag with the
> @@ -6875,6 +6785,21 @@ static inline void set_avr64(int regno, TCGv_i64 src, bool high)
> # define REQUIRE_64BIT(CTX) REQUIRE_INSNS_FLAGS(CTX, 64B)
> #endif
>
> +/*
> + * Helpers for implementing sets of trans_* functions.
> + * Defer the implementation of NAME to FUNC, with optional extra arguments.
> + */
> +#define TRANS(NAME, FUNC, ...) \
> + static bool trans_##NAME(DisasContext *ctx, arg_##NAME *a) \
> + { return FUNC(ctx, a, __VA_ARGS__); }
> +
> +#define TRANS64(NAME, FUNC, ...) \
> + static bool trans_##NAME(DisasContext *ctx, arg_##NAME *a) \
> + { REQUIRE_64BIT(ctx); return FUNC(ctx, a, __VA_ARGS__); }
> +
> +/* TODO: More TRANS* helpers for extra insn_flags checks. */
> +
> +
> #include "decode-insn32.c.inc"
> #include "decode-insn64.c.inc"
> #include "translate/fixedpoint-impl.c.inc"
> @@ -7059,7 +6984,6 @@ GEN_HANDLER2_E(extswsli1, "extswsli", 0x1F, 0x1B, 0x1B, 0x00000000,
> PPC_NONE, PPC2_ISA300),
> #endif
> #if defined(TARGET_PPC64)
> -GEN_HANDLER(ld, 0x3A, 0xFF, 0xFF, 0x00000000, PPC_64B),
> GEN_HANDLER(lq, 0x38, 0xFF, 0xFF, 0x00000000, PPC_64BX),
> GEN_HANDLER(std, 0x3E, 0xFF, 0xFF, 0x00000000, PPC_64B),
> #endif
> @@ -7425,34 +7349,11 @@ GEN_PPC64_R2(rldcr, 0x1E, 0x09),
> GEN_PPC64_R4(rldimi, 0x1E, 0x06),
> #endif
>
> -#undef GEN_LD
> -#undef GEN_LDU
> -#undef GEN_LDUX
> #undef GEN_LDX_E
> -#undef GEN_LDS
> -#define GEN_LD(name, ldop, opc, type) \
> -GEN_HANDLER(name, opc, 0xFF, 0xFF, 0x00000000, type),
> -#define GEN_LDU(name, ldop, opc, type) \
> -GEN_HANDLER(name##u, opc, 0xFF, 0xFF, 0x00000000, type),
> -#define GEN_LDUX(name, ldop, opc2, opc3, type) \
> -GEN_HANDLER(name##ux, 0x1F, opc2, opc3, 0x00000001, type),
> #define GEN_LDX_E(name, ldop, opc2, opc3, type, type2, chk) \
> GEN_HANDLER_E(name##x, 0x1F, opc2, opc3, 0x00000001, type, type2),
> -#define GEN_LDS(name, ldop, op, type) \
> -GEN_LD(name, ldop, op | 0x20, type) \
> -GEN_LDU(name, ldop, op | 0x21, type) \
> -GEN_LDUX(name, ldop, 0x17, op | 0x01, type) \
> -GEN_LDX(name, ldop, 0x17, op | 0x00, type)
>
> -GEN_LDS(lbz, ld8u, 0x02, PPC_INTEGER)
> -GEN_LDS(lha, ld16s, 0x0A, PPC_INTEGER)
> -GEN_LDS(lhz, ld16u, 0x08, PPC_INTEGER)
> -GEN_LDS(lwz, ld32u, 0x00, PPC_INTEGER)
> #if defined(TARGET_PPC64)
> -GEN_LDUX(lwa, ld32s, 0x15, 0x0B, PPC_64B)
> -GEN_LDX(lwa, ld32s, 0x15, 0x0A, PPC_64B)
> -GEN_LDUX(ld, ld64_i64, 0x15, 0x01, PPC_64B)
> -GEN_LDX(ld, ld64_i64, 0x15, 0x00, PPC_64B)
> GEN_LDX_E(ldbr, ld64ur_i64, 0x14, 0x10, PPC_NONE, PPC2_DBRX, CHK_NONE)
>
> /* HV/P7 and later only */
> diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc
> index 96b8c38f60..cb3219c996 100644
> --- a/target/ppc/translate/fixedpoint-impl.c.inc
> +++ b/target/ppc/translate/fixedpoint-impl.c.inc
> @@ -33,6 +33,120 @@ static bool resolve_PLS_D(DisasContext *ctx, arg_PLS_D *a)
> return true;
> }
>
> +/*
> + * Fixed-Point Load/Store Instructions
> + */
> +
> +static bool do_ldst_PLS_D(DisasContext *ctx, arg_PLS_D *a, bool update,
> + bool store, MemOp mop)
> +{
> + TCGv ea;
> +
> + if (!resolve_PLS_D(ctx, a)) {
> + return true;
> + }
> + if (update && (a->ra == 0 || (!store && a->ra == a->rt))) {
> + gen_invalid(ctx);
> + return true;
> + }
> + gen_set_access_type(ctx, ACCESS_INT);
> +
> + ea = tcg_temp_new();
> + if (a->ra) {
> + tcg_gen_addi_tl(ea, cpu_gpr[a->ra], a->si);
> + } else {
> + tcg_gen_movi_tl(ea, a->si);
> + }
> + if (NARROW_MODE(ctx)) {
> + tcg_gen_ext32u_tl(ea, ea);
> + }
> + mop ^= ctx->default_tcg_memop_mask;
> + if (store) {
> + tcg_gen_qemu_st_tl(cpu_gpr[a->rt], ea, ctx->mem_idx, mop);
> + } else {
> + tcg_gen_qemu_ld_tl(cpu_gpr[a->rt], ea, ctx->mem_idx, mop);
> + }
> + if (update) {
> + tcg_gen_mov_tl(cpu_gpr[a->ra], ea);
> + }
> + tcg_temp_free(ea);
> +
> + return true;
> +}
> +
> +static bool do_ldst_X(DisasContext *ctx, arg_X *a, bool update,
> + bool store, MemOp mop)
> +{
> + TCGv ea;
> +
> + if (update && (a->ra == 0 || (!store && a->ra == a->rt))) {
> + gen_invalid(ctx);
> + return true;
> + }
> + gen_set_access_type(ctx, ACCESS_INT);
> +
> + ea = tcg_temp_new();
> + if (a->ra) {
> + tcg_gen_add_tl(ea, cpu_gpr[a->ra], cpu_gpr[a->rb]);
> + } else {
> + tcg_gen_mov_tl(ea, cpu_gpr[a->rb]);
> + }
> + if (NARROW_MODE(ctx)) {
> + tcg_gen_ext32u_tl(ea, ea);
> + }
> + mop ^= ctx->default_tcg_memop_mask;
> + if (store) {
> + tcg_gen_qemu_st_tl(cpu_gpr[a->rt], ea, ctx->mem_idx, mop);
> + } else {
> + tcg_gen_qemu_ld_tl(cpu_gpr[a->rt], ea, ctx->mem_idx, mop);
> + }
> + if (update) {
> + tcg_gen_mov_tl(cpu_gpr[a->ra], ea);
> + }
> + tcg_temp_free(ea);
> +
> + return true;
> +}
> +
The only difference between those two is tcg_gen_addi_tl/tcg_gen_movi_tl
and tcg_gen_add_tl/tcg_gen_mov_tl. We could do this in a single method
if we tcg_const_tl(a->si) in do_ldst_D. I'm not sure about the costs
involved, and we'd need to tcg_temp_free it. If you want to give it a
look, I did some tests on
https://github.com/ppc64/qemu/tree/ferst-tcg-decodetree64 .
Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software Júnior
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH v3 27/30] target/ppc: Move D/DS/X-form integer loads to decodetree
2021-04-30 23:54 ` Matheus K. Ferst
@ 2021-05-01 0:50 ` Richard Henderson
2021-05-03 12:28 ` Matheus K. Ferst
0 siblings, 1 reply; 63+ messages in thread
From: Richard Henderson @ 2021-05-01 0:50 UTC (permalink / raw)
To: Matheus K. Ferst, qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen, david
On 4/30/21 4:54 PM, Matheus K. Ferst wrote:
> The only difference between those two is tcg_gen_addi_tl/tcg_gen_movi_tl and
> tcg_gen_add_tl/tcg_gen_mov_tl. We could do this in a single method if we
> tcg_const_tl(a->si) in do_ldst_D. I'm not sure about the costs involved, and
> we'd need to tcg_temp_free it. If you want to give it a look, I did some tests
> on https://github.com/ppc64/qemu/tree/ferst-tcg-decodetree64 .
I guess it works, but it feels ugly with the comparison vs cpu_gpr[0]. Maybe
pass in rt and ra as integers, and only pass in rb/si as TCGv addend?
The upper-case argument names clash with docs/devel/style.rst.
You can use tcg_constant_tl() to grab a hashed constant that need not be freed.
BTW, I thought I had a comment about PLS being used for both MLS and 8LS. I
guess that must have gotten lost at some point. If you go with your renaming
to MLS, you'll want a different comment about PLD et al.
r~
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH v3 27/30] target/ppc: Move D/DS/X-form integer loads to decodetree
2021-05-01 0:50 ` Richard Henderson
@ 2021-05-03 12:28 ` Matheus K. Ferst
0 siblings, 0 replies; 63+ messages in thread
From: Matheus K. Ferst @ 2021-05-03 12:28 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen, david
On 30/04/2021 21:50, Richard Henderson wrote:
> On 4/30/21 4:54 PM, Matheus K. Ferst wrote:
>> The only difference between those two is
>> tcg_gen_addi_tl/tcg_gen_movi_tl and tcg_gen_add_tl/tcg_gen_mov_tl. We
>> could do this in a single method if we tcg_const_tl(a->si) in
>> do_ldst_D. I'm not sure about the costs involved, and we'd need to
>> tcg_temp_free it. If you want to give it a look, I did some tests on
>> https://github.com/ppc64/qemu/tree/ferst-tcg-decodetree64 .
>
> I guess it works, but it feels ugly with the comparison vs cpu_gpr[0].
> Maybe pass in rt and ra as integers, and only pass in rb/si as TCGv addend?
>
I tried that first and it looked just as weird, but if you prefer I'll
change it back. We could also hide the ugliness in a helper e.g. bool
gpr_is_zero(TCGv).
> The upper-case argument names clash with docs/devel/style.rst.
>
Good call, I'll fix this before push this branch again;
> You can use tcg_constant_tl() to grab a hashed constant that need not be
> freed.
>
Then we could
> return do_ldst(ctx, cpu_gpr[a->rt],
a->ra?cpu->gpr[a->ra]:tcg_const_tl(0), ...
At the cost of always calling tcg_gen_add_tl and repeating the RA=0
logic in do_ldst_X and do_ldst_D.
> BTW, I thought I had a comment about PLS being used for both MLS and
> 8LS. I guess that must have gotten lost at some point. If you go with
> your renaming to MLS, you'll want a different comment about PLD et al.
>
>
> r~
Don't mind that, it was a bad idea, I gave up on this change while
rebasing v3.
Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software Júnior
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH v3 28/30] target/ppc: Implement prefixed integer load instructions
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (26 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 27/30] target/ppc: Move D/DS/X-form integer loads to decodetree Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 1:15 ` [PATCH v3 29/30] target/ppc: Move D/DS/X-form integer stores to decodetree Richard Henderson
` (2 subsequent siblings)
30 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/insn64.decode | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/target/ppc/insn64.decode b/target/ppc/insn64.decode
index 5a82ce375e..4198e5c8f3 100644
--- a/target/ppc/insn64.decode
+++ b/target/ppc/insn64.decode
@@ -29,6 +29,21 @@
...... rt:5 ra:5 ................ \
&PLS_D si=%pls_si
+### Fixed-Point Load Instructions
+
+LBZ 000001 10 0--.-- .................. \
+ 100010 ..... ..... ................ @PLS_D
+LHZ 000001 10 0--.-- .................. \
+ 101000 ..... ..... ................ @PLS_D
+LHA 000001 10 0--.-- .................. \
+ 101010 ..... ..... ................ @PLS_D
+LWZ 000001 10 0--.-- .................. \
+ 100000 ..... ..... ................ @PLS_D
+LWA 000001 00 0--.-- .................. \
+ 101001 ..... ..... ................ @PLS_D
+LD 000001 00 0--.-- .................. \
+ 111001 ..... ..... ................ @PLS_D
+
### Fixed-Point Arithmetic Instructions
ADDI 000001 10 0--.-- .................. \
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH v3 29/30] target/ppc: Move D/DS/X-form integer stores to decodetree
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (27 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 28/30] target/ppc: Implement prefixed integer load instructions Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 1:15 ` [PATCH v3 30/30] target/ppc: Implement prefixed integer store instructions Richard Henderson
2021-04-30 1:48 ` [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions no-reply
30 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
These are all connected by macros in the legacy decoding.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/insn32.decode | 22 ++++++
target/ppc/translate.c | 83 +---------------------
target/ppc/translate/fixedpoint-impl.c.inc | 24 +++++++
3 files changed, 48 insertions(+), 81 deletions(-)
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index 1c1b4620fc..7d35f61e45 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -63,6 +63,28 @@ LDU 111010 ..... ..... ..............01 @PLS_DS
LDX 011111 ..... ..... ..... 0000010101 - @X
LDUX 011111 ..... ..... ..... 0000110101 - @X
+### Fixed-Point Store Instructions
+
+STB 100110 ..... ..... ................ @PLS_D
+STBU 100111 ..... ..... ................ @PLS_D
+STBX 011111 ..... ..... ..... 0011010111 - @X
+STBUX 011111 ..... ..... ..... 0011110111 - @X
+
+STH 101100 ..... ..... ................ @PLS_D
+STHU 101101 ..... ..... ................ @PLS_D
+STHX 011111 ..... ..... ..... 0110010111 - @X
+STHUX 011111 ..... ..... ..... 0110110111 - @X
+
+STW 100100 ..... ..... ................ @PLS_D
+STWU 100101 ..... ..... ................ @PLS_D
+STWX 011111 ..... ..... ..... 0010010111 - @X
+STWUX 011111 ..... ..... ..... 0010110111 - @X
+
+STD 111110 ..... ..... ..............00 @PLS_DS
+STDU 111110 ..... ..... ..............01 @PLS_DS
+STDX 011111 ..... ..... ..... 0010010101 - @X
+STDUX 011111 ..... ..... ..... 0010110101 - @X
+
### Fixed-Point Arithmetic Instructions
ADDI 001110 ..... ..... ................ @PLS_D
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 1fdb501ee9..ad32fcc740 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -2463,7 +2463,9 @@ static void glue(gen_qemu_, stop)(DisasContext *ctx, \
tcg_gen_qemu_st_tl(val, addr, ctx->mem_idx, op); \
}
+#if defined(TARGET_PPC64) || !defined(CONFIG_USER_ONLY)
GEN_QEMU_STORE_TL(st8, DEF_MEMOP(MO_UB))
+#endif
GEN_QEMU_STORE_TL(st16, DEF_MEMOP(MO_UW))
GEN_QEMU_STORE_TL(st32, DEF_MEMOP(MO_UL))
@@ -2596,52 +2598,6 @@ static void gen_lq(DisasContext *ctx)
#endif
/*** Integer store ***/
-#define GEN_ST(name, stop, opc, type) \
-static void glue(gen_, name)(DisasContext *ctx) \
-{ \
- TCGv EA; \
- gen_set_access_type(ctx, ACCESS_INT); \
- EA = tcg_temp_new(); \
- gen_addr_imm_index(ctx, EA, 0); \
- gen_qemu_##stop(ctx, cpu_gpr[rS(ctx->opcode)], EA); \
- tcg_temp_free(EA); \
-}
-
-#define GEN_STU(name, stop, opc, type) \
-static void glue(gen_, stop##u)(DisasContext *ctx) \
-{ \
- TCGv EA; \
- if (unlikely(rA(ctx->opcode) == 0)) { \
- gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL); \
- return; \
- } \
- gen_set_access_type(ctx, ACCESS_INT); \
- EA = tcg_temp_new(); \
- if (type == PPC_64B) \
- gen_addr_imm_index(ctx, EA, 0x03); \
- else \
- gen_addr_imm_index(ctx, EA, 0); \
- gen_qemu_##stop(ctx, cpu_gpr[rS(ctx->opcode)], EA); \
- tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], EA); \
- tcg_temp_free(EA); \
-}
-
-#define GEN_STUX(name, stop, opc2, opc3, type) \
-static void glue(gen_, name##ux)(DisasContext *ctx) \
-{ \
- TCGv EA; \
- if (unlikely(rA(ctx->opcode) == 0)) { \
- gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL); \
- return; \
- } \
- gen_set_access_type(ctx, ACCESS_INT); \
- EA = tcg_temp_new(); \
- gen_addr_reg_index(ctx, EA); \
- gen_qemu_##stop(ctx, cpu_gpr[rS(ctx->opcode)], EA); \
- tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], EA); \
- tcg_temp_free(EA); \
-}
-
#define GEN_STX_E(name, stop, opc2, opc3, type, type2, chk) \
static void glue(gen_, name##x)(DisasContext *ctx) \
{ \
@@ -2659,19 +2615,6 @@ static void glue(gen_, name##x)(DisasContext *ctx) \
#define GEN_STX_HVRM(name, stop, opc2, opc3, type) \
GEN_STX_E(name, stop, opc2, opc3, type, PPC_NONE, CHK_HVRM)
-#define GEN_STS(name, stop, op, type) \
-GEN_ST(name, stop, op | 0x20, type); \
-GEN_STU(name, stop, op | 0x21, type); \
-GEN_STUX(name, stop, 0x17, op | 0x01, type); \
-GEN_STX(name, stop, 0x17, op | 0x00, type)
-
-/* stb stbu stbux stbx */
-GEN_STS(stb, st8, 0x06, PPC_INTEGER);
-/* sth sthu sthux sthx */
-GEN_STS(sth, st16, 0x0C, PPC_INTEGER);
-/* stw stwu stwux stwx */
-GEN_STS(stw, st32, 0x04, PPC_INTEGER);
-
#define GEN_STEPX(name, stop, opc2, opc3) \
static void glue(gen_, name##epx)(DisasContext *ctx) \
{ \
@@ -2693,8 +2636,6 @@ GEN_STEPX(std, DEF_MEMOP(MO_Q), 0x1d, 0x04)
#endif
#if defined(TARGET_PPC64)
-GEN_STUX(std, st64_i64, 0x15, 0x05, PPC_64B);
-GEN_STX(std, st64_i64, 0x15, 0x04, PPC_64B);
GEN_STX_HVRM(stdcix, st64_i64, 0x15, 0x1f, PPC_CILDST)
GEN_STX_HVRM(stwcix, st32, 0x15, 0x1c, PPC_CILDST)
GEN_STX_HVRM(sthcix, st16, 0x15, 0x1d, PPC_CILDST)
@@ -7378,31 +7319,11 @@ GEN_LDEPX(lw, DEF_MEMOP(MO_UL), 0x1F, 0x00)
GEN_LDEPX(ld, DEF_MEMOP(MO_Q), 0x1D, 0x00)
#endif
-#undef GEN_ST
-#undef GEN_STU
-#undef GEN_STUX
#undef GEN_STX_E
-#undef GEN_STS
-#define GEN_ST(name, stop, opc, type) \
-GEN_HANDLER(name, opc, 0xFF, 0xFF, 0x00000000, type),
-#define GEN_STU(name, stop, opc, type) \
-GEN_HANDLER(stop##u, opc, 0xFF, 0xFF, 0x00000000, type),
-#define GEN_STUX(name, stop, opc2, opc3, type) \
-GEN_HANDLER(name##ux, 0x1F, opc2, opc3, 0x00000001, type),
#define GEN_STX_E(name, stop, opc2, opc3, type, type2, chk) \
GEN_HANDLER_E(name##x, 0x1F, opc2, opc3, 0x00000000, type, type2),
-#define GEN_STS(name, stop, op, type) \
-GEN_ST(name, stop, op | 0x20, type) \
-GEN_STU(name, stop, op | 0x21, type) \
-GEN_STUX(name, stop, 0x17, op | 0x01, type) \
-GEN_STX(name, stop, 0x17, op | 0x00, type)
-GEN_STS(stb, st8, 0x06, PPC_INTEGER)
-GEN_STS(sth, st16, 0x0C, PPC_INTEGER)
-GEN_STS(stw, st32, 0x04, PPC_INTEGER)
#if defined(TARGET_PPC64)
-GEN_STUX(std, st64_i64, 0x15, 0x05, PPC_64B)
-GEN_STX(std, st64_i64, 0x15, 0x04, PPC_64B)
GEN_STX_E(stdbr, st64r_i64, 0x14, 0x14, PPC_NONE, PPC2_DBRX, CHK_NONE)
GEN_STX_HVRM(stdcix, st64_i64, 0x15, 0x1f, PPC_CILDST)
GEN_STX_HVRM(stwcix, st32, 0x15, 0x1c, PPC_CILDST)
diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc
index cb3219c996..6d57f0038b 100644
--- a/target/ppc/translate/fixedpoint-impl.c.inc
+++ b/target/ppc/translate/fixedpoint-impl.c.inc
@@ -143,6 +143,30 @@ TRANS64(LDX, do_ldst_X, false, false, MO_Q)
TRANS64(LDU, do_ldst_PLS_D, true, false, MO_Q)
TRANS64(LDUX, do_ldst_X, true, false, MO_Q)
+/* Store Byte */
+TRANS(STB, do_ldst_PLS_D, false, true, MO_UB)
+TRANS(STBX, do_ldst_X, false, true, MO_UB)
+TRANS(STBU, do_ldst_PLS_D, true, true, MO_UB)
+TRANS(STBUX, do_ldst_X, true, true, MO_UB)
+
+/* Store Halfword */
+TRANS(STH, do_ldst_PLS_D, false, true, MO_UW)
+TRANS(STHX, do_ldst_X, false, true, MO_UW)
+TRANS(STHU, do_ldst_PLS_D, true, true, MO_UW)
+TRANS(STHUX, do_ldst_X, true, true, MO_UW)
+
+/* Store Word */
+TRANS(STW, do_ldst_PLS_D, false, true, MO_UL)
+TRANS(STWX, do_ldst_X, false, true, MO_UL)
+TRANS(STWU, do_ldst_PLS_D, true, true, MO_UL)
+TRANS(STWUX, do_ldst_X, true, true, MO_UL)
+
+/* Store Doubleword */
+TRANS64(STD, do_ldst_PLS_D, false, true, MO_Q)
+TRANS64(STDX, do_ldst_X, false, true, MO_Q)
+TRANS64(STDU, do_ldst_PLS_D, true, true, MO_Q)
+TRANS64(STDUX, do_ldst_X, true, true, MO_Q)
+
/*
* Fixed-Point Arithmetic Instructions
*/
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH v3 30/30] target/ppc: Implement prefixed integer store instructions
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (28 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 29/30] target/ppc: Move D/DS/X-form integer stores to decodetree Richard Henderson
@ 2021-04-30 1:15 ` Richard Henderson
2021-04-30 1:48 ` [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions no-reply
30 siblings, 0 replies; 63+ messages in thread
From: Richard Henderson @ 2021-04-30 1:15 UTC (permalink / raw)
To: qemu-devel
Cc: f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/insn64.decode | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/target/ppc/insn64.decode b/target/ppc/insn64.decode
index 4198e5c8f3..7a71a7a3bb 100644
--- a/target/ppc/insn64.decode
+++ b/target/ppc/insn64.decode
@@ -44,6 +44,18 @@ LWA 000001 00 0--.-- .................. \
LD 000001 00 0--.-- .................. \
111001 ..... ..... ................ @PLS_D
+### Fixed-Point Store Instructions
+
+STW 000001 10 0--.-- .................. \
+ 100100 ..... ..... ................ @PLS_D
+STB 000001 10 0--.-- .................. \
+ 100110 ..... ..... ................ @PLS_D
+STH 000001 10 0--.-- .................. \
+ 101100 ..... ..... ................ @PLS_D
+
+STD 000001 00 0--.-- .................. \
+ 111101 ..... ..... ................ @PLS_D
+
### Fixed-Point Arithmetic Instructions
ADDI 000001 10 0--.-- .................. \
--
2.25.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions
2021-04-30 1:15 [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions Richard Henderson
` (29 preceding siblings ...)
2021-04-30 1:15 ` [PATCH v3 30/30] target/ppc: Implement prefixed integer store instructions Richard Henderson
@ 2021-04-30 1:48 ` no-reply
30 siblings, 0 replies; 63+ messages in thread
From: no-reply @ 2021-04-30 1:48 UTC (permalink / raw)
To: richard.henderson
Cc: qemu-devel, f4bug, luis.pires, qemu-ppc, lagarcia, bruno.larsen,
matheus.ferst, david
Patchew URL: https://patchew.org/QEMU/20210430011543.1017113-1-richard.henderson@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210430011543.1017113-1-richard.henderson@linaro.org
Subject: [PATCH v3 00/30] Base for adding PowerPC 64-bit instructions
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20210430011543.1017113-1-richard.henderson@linaro.org -> patchew/20210430011543.1017113-1-richard.henderson@linaro.org
Switched to a new branch 'test'
94d954b target/ppc: Implement prefixed integer store instructions
d2b6c00 target/ppc: Move D/DS/X-form integer stores to decodetree
3b6ae7e target/ppc: Implement prefixed integer load instructions
7785d8b target/ppc: Move D/DS/X-form integer loads to decodetree
60fd320 target/ppc: Implement PNOP
6d3431c target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI
9cbf606 target/ppc: Move page crossing check to ppc_tr_translate_insn
f04f977 target/ppc: Add infrastructure for prefixed insns
efe0f1b target/ppc: Introduce macros to check isa extensions
69d9dd3 target/ppc: Use translator_loop_temp_check
2aba16a target/ppc: Mark helper_raise_exception* as noreturn
3b0336d target/ppc: Tidy exception vs exit_tb
d9e4ef4 target/ppc: Move single-step check to ppc_tr_tb_stop
27cd7b1 target/ppc: Remove DisasContext.exception
1241c8b target/ppc: Replace POWERPC_EXCP_BRANCH with DISAS_NORETURN
79906ac target/ppc: Replace POWERPC_EXCP_STOP with DISAS_EXIT_UPDATE
cf2d292 target/ppc: Introduce gen_icount_io_start
434992b target/ppc: Remove unnecessary gen_io_end calls
4c7c9fe target/ppc: Replace POWERPC_EXCP_SYNC with DISAS_EXIT
e4f28a4 target/ppc: Introduce DISAS_{EXIT,CHAIN}{,_UPDATE}
59c8dec target/ppc: Simplify gen_debug_exception
33ef476 target/ppc: Remove special case for POWERPC_EXCP_TRAP
a899437 target/ppc: Remove special case for POWERPC_SYSCALL
65217f2 target/ppc: Move DISAS_NORETURN setting into gen_exception*
68b60d8 target/ppc: Split out decode_legacy
91d5d66 target/ppc: Add cia field to DisasContext
8b45cf3 decodetree: Extend argument set syntax to allow types
2797541 decodetree: Add support for 64-bit instructions
1b1c946 decodetree: More use of f-strings
fd4892f decodetree: Introduce whex and whexC helpers
=== OUTPUT BEGIN ===
1/30 Checking commit fd4892f09eff (decodetree: Introduce whex and whexC helpers)
ERROR: line over 90 characters
#51: FILE: scripts/decodetree.py:495:
+ output(ind, f'if ((insn & {whexC(innermask)}) == {whexC(innerbits)}) {{\n')
WARNING: line over 80 characters
#52: FILE: scripts/decodetree.py:496:
+ output(ind, f' /* {str_match_bits(p.fixedbits, p.fixedmask)} */\n')
total: 1 errors, 1 warnings, 136 lines checked
Patch 1/30 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/30 Checking commit 1b1c946ade32 (decodetree: More use of f-strings)
3/30 Checking commit 279754143f3b (decodetree: Add support for 64-bit instructions)
WARNING: line over 80 characters
#74: FILE: scripts/decodetree.py:236:
+ ret = f'deposit{bitop_width}({ret}, {pos}, {bitop_width - pos}, {ext})'
total: 0 errors, 1 warnings, 63 lines checked
Patch 3/30 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/30 Checking commit 8b45cf3a2c2d (decodetree: Extend argument set syntax to allow types)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#155:
new file mode 100644
total: 0 errors, 1 warnings, 121 lines checked
Patch 4/30 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/30 Checking commit 91d5d66281c8 (target/ppc: Add cia field to DisasContext)
6/30 Checking commit 68b60d843565 (target/ppc: Split out decode_legacy)
7/30 Checking commit 65217f275156 (target/ppc: Move DISAS_NORETURN setting into gen_exception*)
8/30 Checking commit a899437b0743 (target/ppc: Remove special case for POWERPC_SYSCALL)
9/30 Checking commit 33ef476619c9 (target/ppc: Remove special case for POWERPC_EXCP_TRAP)
10/30 Checking commit 59c8dec6fc93 (target/ppc: Simplify gen_debug_exception)
11/30 Checking commit e4f28a4f9529 (target/ppc: Introduce DISAS_{EXIT,CHAIN}{,_UPDATE})
12/30 Checking commit 4c7c9fedf741 (target/ppc: Replace POWERPC_EXCP_SYNC with DISAS_EXIT)
13/30 Checking commit 434992beb095 (target/ppc: Remove unnecessary gen_io_end calls)
14/30 Checking commit cf2d2924fa23 (target/ppc: Introduce gen_icount_io_start)
15/30 Checking commit 79906ac9717a (target/ppc: Replace POWERPC_EXCP_STOP with DISAS_EXIT_UPDATE)
16/30 Checking commit 1241c8bf8c1f (target/ppc: Replace POWERPC_EXCP_BRANCH with DISAS_NORETURN)
17/30 Checking commit 27cd7b154dca (target/ppc: Remove DisasContext.exception)
18/30 Checking commit d9e4ef4ce3ce (target/ppc: Move single-step check to ppc_tr_tb_stop)
19/30 Checking commit 3b0336de441d (target/ppc: Tidy exception vs exit_tb)
20/30 Checking commit 2aba16a9994f (target/ppc: Mark helper_raise_exception* as noreturn)
21/30 Checking commit 69d9dd38fc6d (target/ppc: Use translator_loop_temp_check)
22/30 Checking commit efe0f1bf6c8d (target/ppc: Introduce macros to check isa extensions)
23/30 Checking commit f04f977efd8b (target/ppc: Add infrastructure for prefixed insns)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#32:
new file mode 100644
total: 0 errors, 1 warnings, 134 lines checked
Patch 23/30 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
24/30 Checking commit 9cbf60690a9f (target/ppc: Move page crossing check to ppc_tr_translate_insn)
25/30 Checking commit 6d3431c00f48 (target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI)
26/30 Checking commit 60fd320f9d79 (target/ppc: Implement PNOP)
27/30 Checking commit 7785d8bc647c (target/ppc: Move D/DS/X-form integer loads to decodetree)
ERROR: spaces required around that '*' (ctx:WxV)
#220: FILE: target/ppc/translate.c:6793:
+ static bool trans_##NAME(DisasContext *ctx, arg_##NAME *a) \
^
ERROR: spaces required around that '*' (ctx:WxV)
#224: FILE: target/ppc/translate.c:6797:
+ static bool trans_##NAME(DisasContext *ctx, arg_##NAME *a) \
^
total: 2 errors, 0 warnings, 361 lines checked
Patch 27/30 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
28/30 Checking commit 3b6ae7e1d972 (target/ppc: Implement prefixed integer load instructions)
29/30 Checking commit d2b6c00190e9 (target/ppc: Move D/DS/X-form integer stores to decodetree)
30/30 Checking commit 94d954b2f59f (target/ppc: Implement prefixed integer store instructions)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210430011543.1017113-1-richard.henderson@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 63+ messages in thread