[01/67] decodetree: Allow !function with no input bits

Message ID 20190726175032.6769-2-richard.henderson@linaro.org
State New
Headers show
Series
  • target/arm: Convert aa32 base isa to decodetree
Related show

Commit Message

Richard Henderson July 26, 2019, 5:49 p.m.
With this, we can have the function return a value from the DisasContext.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 scripts/decodetree.py             | 5 ++++-
 tests/decode/succ_function.decode | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)
 create mode 100644 tests/decode/succ_function.decode

-- 
2.17.1

Comments

Peter Maydell July 29, 2019, 1:43 p.m. | #1
On Fri, 26 Jul 2019 at 18:50, Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> With this, we can have the function return a value from the DisasContext.

>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  scripts/decodetree.py             | 5 ++++-

>  tests/decode/succ_function.decode | 2 ++

>  2 files changed, 6 insertions(+), 1 deletion(-)

>  create mode 100644 tests/decode/succ_function.decode

>

> diff --git a/scripts/decodetree.py b/scripts/decodetree.py

> index d7a59d63ac..4259d87a95 100755

> --- a/scripts/decodetree.py

> +++ b/scripts/decodetree.py

> @@ -195,7 +195,10 @@ class MultiField:

>      """Class representing a compound instruction field"""

>      def __init__(self, subs, mask):

>          self.subs = subs

> -        self.sign = subs[0].sign

> +        if len(subs):

> +            self.sign = subs[0].sign

> +        else:

> +            self.sign = 0

>          self.mask = mask

>

>      def __str__(self):

> diff --git a/tests/decode/succ_function.decode b/tests/decode/succ_function.decode

> new file mode 100644

> index 0000000000..632a9de252

> --- /dev/null

> +++ b/tests/decode/succ_function.decode

> @@ -0,0 +1,2 @@

> +%foo  !function=foo

> +foo   00000000000000000000000000000000 %foo

> --

> 2.17.1


Could you also update the documentation in docs/devel/decodetree.rst ?

This code change looks like it also now allows definitions
of fields that specify nothing at all (ie there's no check
that a field definition with no "unnamed_field" parts has
a !function specifier) -- what do they do, or should they
be made syntax errors ?

Is one of these functions which just returns a constant
from no input bits still a "static int func(DisasContext *s, int x)"
taking a pointless input argument, or is it now a
"static int func(DisasContext *s)" ? I guess from the fact
this code doesn't change the way a call is output that it
is the former, but would the latter be cleaner ?
(This would probably be implemented something like allowing
FunctionField to be passed a base == None instead of
allowing MultiFields with len(subs) == 0.)

thanks
-- PMM
Richard Henderson July 30, 2019, 1:30 a.m. | #2
On 7/29/19 6:43 AM, Peter Maydell wrote:
> On Fri, 26 Jul 2019 at 18:50, Richard Henderson

> <richard.henderson@linaro.org> wrote:

>>

>> With this, we can have the function return a value from the DisasContext.

>>

>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

>> ---

>>  scripts/decodetree.py             | 5 ++++-

>>  tests/decode/succ_function.decode | 2 ++

>>  2 files changed, 6 insertions(+), 1 deletion(-)

>>  create mode 100644 tests/decode/succ_function.decode

>>

>> diff --git a/scripts/decodetree.py b/scripts/decodetree.py

>> index d7a59d63ac..4259d87a95 100755

>> --- a/scripts/decodetree.py

>> +++ b/scripts/decodetree.py

>> @@ -195,7 +195,10 @@ class MultiField:

>>      """Class representing a compound instruction field"""

>>      def __init__(self, subs, mask):

>>          self.subs = subs

>> -        self.sign = subs[0].sign

>> +        if len(subs):

>> +            self.sign = subs[0].sign

>> +        else:

>> +            self.sign = 0

>>          self.mask = mask

>>

>>      def __str__(self):

>> diff --git a/tests/decode/succ_function.decode b/tests/decode/succ_function.decode

>> new file mode 100644

>> index 0000000000..632a9de252

>> --- /dev/null

>> +++ b/tests/decode/succ_function.decode

>> @@ -0,0 +1,2 @@

>> +%foo  !function=foo

>> +foo   00000000000000000000000000000000 %foo

>> --

>> 2.17.1

> 

> Could you also update the documentation in docs/devel/decodetree.rst ?

> 

> This code change looks like it also now allows definitions

> of fields that specify nothing at all (ie there's no check

> that a field definition with no "unnamed_field" parts has

> a !function specifier) -- what do they do, or should they

> be made syntax errors ?


Ah good point.  Should be syntax errors.

> Is one of these functions which just returns a constant

> from no input bits still a "static int func(DisasContext *s, int x)"

> taking a pointless input argument, or is it now a

> "static int func(DisasContext *s)" ? I guess from the fact

> this code doesn't change the way a call is output that it

> is the former, but would the latter be cleaner ?


Right on both counts.  Because of how the loop in MultiField is set up, x will
always be given 0.  I'll see about cleaning this up.

In the meantime, fyi, this is used for setting the S bit for thumb1 insns,
where S=0 iff the insn is within an IT block.


r~

Patch

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index d7a59d63ac..4259d87a95 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -195,7 +195,10 @@  class MultiField:
     """Class representing a compound instruction field"""
     def __init__(self, subs, mask):
         self.subs = subs
-        self.sign = subs[0].sign
+        if len(subs):
+            self.sign = subs[0].sign
+        else:
+            self.sign = 0
         self.mask = mask
 
     def __str__(self):
diff --git a/tests/decode/succ_function.decode b/tests/decode/succ_function.decode
new file mode 100644
index 0000000000..632a9de252
--- /dev/null
+++ b/tests/decode/succ_function.decode
@@ -0,0 +1,2 @@ 
+%foo  !function=foo
+foo   00000000000000000000000000000000 %foo