On 01/14/2013 10:13 PM, Steven J. Hill wrote:
From: "Steven J. Hill" <sjhill@xxxxxxxx>
This patch shows the use of macros in place of 'union mips_instruction'
type.
Why? What are the benefits of doing this?
I converted all usages of 'j_format' and 'r_format' to show how
the code and macros could look and be defined. I have tested these
changes on big and little endian platforms.
I want input from everyone, please!!! I want consensus on the macro
definitions, placement of parenthesis for them, spacing in the header
file, etc. This is your chance to be completely anal and have fun
arguments over how things should be. I would also like input on how
the maintainers would like the patchsets to look like. For example:
[1/X] - Convert 'j_format'
[2/X] - Convert 'r_format'
[3/X] - Convert 'f_format'
[4/X] - Convert 'u_format'
...
[X/X] - Remove usage of 'union mips_instruction' type completely.
Also, I noticed 'p_format' is not used anywhere. Can we kill it? Be
picky and help with this conversion. Thanks.
Signed-off-by: Steven J. Hill <sjhill@xxxxxxxx>
---
arch/mips/include/asm/inst.h | 66 +++++++++++-----------------------------
arch/mips/kernel/branch.c | 13 ++++----
arch/mips/kernel/jump_label.c | 10 +++---
arch/mips/kernel/kgdb.c | 10 ++----
arch/mips/kernel/kprobes.c | 18 +++++------
arch/mips/kernel/process.c | 10 +++---
arch/mips/oprofile/backtrace.c | 2 +-
7 files changed, 46 insertions(+), 83 deletions(-)
diff --git a/arch/mips/include/asm/inst.h b/arch/mips/include/asm/inst.h
index ab84064..856b14e 100644
--- a/arch/mips/include/asm/inst.h
+++ b/arch/mips/include/asm/inst.h
@@ -192,15 +192,27 @@ enum lx_func {
lbx_op = 0x16,
};
+#define INSN_OPCODE(insn) (insn >> 26)
+#define INSN_RS(insn) ((insn >> 21) & 0x1f)
+#define INSN_RT(insn) ((insn >> 16) & 0x1f)
+#define INSN_RD(insn) ((insn >> 11) & 0x1f)
+#define INSN_RE(insn) ((insn >> 6) & 0x1f)
+#define INSN_FUNC(insn) (insn & 0x0000003f)
+
+#define J_INSN(op,target) ((op << 26) | target)
What is the type of J_INSN()? What happens if target overflows into the
'op' field?
+#define J_INSN_TARGET(insn) (insn & 0x03ffffff)
+#define R_INSN(op,rs,rt,rd,re,func) ((op << 26) | (rs << 21) | \
+ (rt << 16) | (rd << 11) | \
+ (re << 6) | func)
+#define F_INSN(op,fmt,rt,rd,re,func) R_INSN(op,fmt,rt,rd,re,func)
+#define F_INSN_FMT(insn) INSN_RS(insn)
+#define U_INSN(op,rs,uimm) ((op << 26) | (rs << 21) | uimmediate)
[...]
+ unsigned int n_insn = insn.word;
I don't like that the width of an insn is not obvious by looking at the
code.
Can we, assuming we merge something like this, make it something like
u32, or insn_t? I'm not sure which is better.
[...]
David Daney
