* [Qemu-devel] Add CMP2 instruction
@ 2014-11-07 10:14 Guo, Lei
2014-11-07 11:10 ` Alex Bennée
` (5 more replies)
0 siblings, 6 replies; 10+ messages in thread
From: Guo, Lei @ 2014-11-07 10:14 UTC (permalink / raw)
To: qemu-devel@nongnu.org; +Cc: Wang, Chunping
[-- Attachment #1: Type: text/plain, Size: 3832 bytes --]
This patch aims to add CMP2 instruction for m68k family.
Description: Compares the value in Rn to each bound. The effective address contains the
bounds pair: upper bound following the lower bound. For signed comparisons, the
arithmetically smaller value should be used as the lower bound. For unsigned
comparisons, the logically smaller value should be the lower bound.
The size of the data and the bounds can be specified as byte, word, or long. If Rn is a
data register and the operation size is byte or word, only the appropriate low-order part
of Rn is checked. If Rn is an address register and the operation size is byte or word,
the bounds operands are sign-extended to 32 bits, and the resultant operands are
compared to the full 32 bits of An.
If the upper bound equals the lower bound, the valid range is a single value.
signed-off-by: Guolei <guol-fnst@cn.fujitsu.com <mailto:guol-fnst@cn.fujitsu.com%20> >
---
target-m68k/translate.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index efd4cfc..8c09248 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -1931,6 +1931,79 @@ DISAS_INSN(ff1)
gen_helper_ff1(reg, reg);
}
+DISAS_INSN(cmp2)
+{
+ TCGv src;
+ TCGv reg;
+ int opsize;
+ int l1, l2, l3, l4;
+ TCGv lower, upper;
+ uint16_t ext;
+
+ switch ((insn >> 9) & 3) {
+ case 0:
+ opsize = OS_BYTE;
+ break;
+ case 1:
+ opsize = OS_WORD;
+ break;
+ case 2:
+ opsize = OS_LONG;
+ break;
+ default:
+ gen_exception(s, s->pc, EXCP_ILLEGAL);
+ return;
+ }
+
+ SRC_EA(env, src, opsize, -1, NULL);
+ lower = tcg_temp_new();
+ upper = tcg_temp_new();
+ tcg_gen_shri_i32(lower, lower, 16);
+ tcg_gen_andi_i32(lower, lower, 0xffff);
+ tcg_gen_andi_i32(upper, upper, 0xffff);
+
+ ext = cpu_ldsw_code(env, s->pc);
+ s->pc += 2;
+ if (ext & 0x8000) {
+ reg = AREG(ext, 12);
+ } else {
+ reg = DREG(ext, 12);
+ if (opsize == OS_BYTE){
+ tcg_gen_andi_i32(reg, reg, 0xf);
+ } else if (opsize == OS_WORD) {
+ tcg_gen_andi_i32(reg, reg, 0xff);
+ }
+ }
+
+ l1 = gen_new_label();
+ l2 = gen_new_label();
+ l3 = gen_new_label();
+ l4 = gen_new_label();
+
+ tcg_gen_brcond_i32(TCG_COND_NE, reg, lower, l1);
+ s->cc_op = CC_OP_FLAGS;
+ tcg_gen_ori_i32(QREG_CC_DEST, QREG_CC_DEST, CCF_Z);
+ tcg_gen_br(l4);
+
+ gen_set_label(l1);
+ tcg_gen_brcond_i32(TCG_COND_NE, reg, upper, l2);
+ s->cc_op = CC_OP_FLAGS;
+ tcg_gen_ori_i32(QREG_CC_DEST, QREG_CC_DEST, CCF_Z);
+ tcg_gen_br(l4);
+
+ gen_set_label(l2);
+ tcg_gen_brcond_i32(TCG_COND_GT, reg, lower, l3);
+ s->cc_op = CC_OP_FLAGS;
+ tcg_gen_ori_i32(QREG_CC_DEST, QREG_CC_DEST, CCF_C);
+ tcg_gen_br(l4);
+
+ gen_set_label(l3);
+ tcg_gen_brcond_i32(TCG_COND_LT, reg, upper, l4);
+ s->cc_op = CC_OP_FLAGS;
+ tcg_gen_ori_i32(QREG_CC_DEST, QREG_CC_DEST, CCF_C);
+ gen_set_label(l4);
+}
+
static TCGv gen_get_sr(DisasContext *s)
{
TCGv ccr;
@@ -2847,6 +2920,7 @@ void register_m68k_insns (CPUM68KState *env)
INSN(bitop_reg, 01c0, f1c0, CF_ISA_A);
INSN(arith_im, 0280, fff8, CF_ISA_A);
INSN(byterev, 02c0, fff8, CF_ISA_APLUSC);
+ INSN(cmp2, 02c0, ffff, CF_ISA_A);
INSN(arith_im, 0480, fff8, CF_ISA_A);
INSN(ff1, 04c0, fff8, CF_ISA_APLUSC);
INSN(arith_im, 0680, fff8, CF_ISA_A);
--
1.7.9.5
[-- Attachment #2: Type: text/html, Size: 16082 bytes --]
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Add CMP2 instruction
2014-11-07 10:14 [Qemu-devel] Add CMP2 instruction Guo, Lei
@ 2014-11-07 11:10 ` Alex Bennée
2014-11-07 11:15 ` Andreas Färber
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Alex Bennée @ 2014-11-07 11:10 UTC (permalink / raw)
To: Guo, Lei; +Cc: qemu-devel@nongnu.org, Wang, Chunping
Guo, Lei <guol-fnst@cn.fujitsu.com> writes:
> This patch aims to add CMP2 instruction for m68k family.
>
> Description: Compares the value in Rn to each bound. The effective address contains the
> bounds pair: upper bound following the lower bound. For signed comparisons, the
> arithmetically smaller value should be used as the lower bound. For unsigned
> comparisons, the logically smaller value should be the lower bound.
> The size of the data and the bounds can be specified as byte, word, or long. If Rn is a
> data register and the operation size is byte or word, only the appropriate low-order part
> of Rn is checked. If Rn is an address register and the operation size is byte or word,
> the bounds operands are sign-extended to 32 bits, and the resultant operands are
> compared to the full 32 bits of An.
> If the upper bound equals the lower bound, the valid range is a single value.
>
>
> signed-off-by: Guolei <guol-fnst@cn.fujitsu.com
> <mailto:guol-fnst@cn.fujitsu.com%20> >
incorrect Signed-off-by: format
>
> ---
> target-m68k/translate.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 74 insertions(+)
I'm fairly sure this fails scripts/checkpatch.pl but I was unable to run
it as your email seems to be html with a base64 uuencoded blob in it.
Please check your patch with checkpatch.pl before re-sending.
Also please fix your mailer to send plain text emails or better yet use
git-send-email to send the patch.
--
Alex Bennée
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Add CMP2 instruction
2014-11-07 10:14 [Qemu-devel] Add CMP2 instruction Guo, Lei
2014-11-07 11:10 ` Alex Bennée
@ 2014-11-07 11:15 ` Andreas Färber
2014-11-10 3:21 ` [Qemu-devel] 答复: " Guo, Lei
2014-11-07 11:26 ` [Qemu-devel] " Richard Henderson
` (3 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Andreas Färber @ 2014-11-07 11:15 UTC (permalink / raw)
To: Guo, Lei, qemu-devel@nongnu.org
Cc: Thomas Huth, Andreas Schwab, Wang, Chunping, Laurent Vivier
Hi,
Am 07.11.2014 um 11:14 schrieb Guo, Lei:
> This patch aims to add CMP2 instruction for m68k family.
>
>
>
> *Description: *Compares the value in Rn to each bound. The effective
> address contains the
>
> bounds pair: upper bound following the lower bound. For signed
> comparisons, the
>
> arithmetically smaller value should be used as the lower bound. For unsigned
>
> comparisons, the logically smaller value should be the lower bound.
>
> The size of the data and the bounds can be specified as byte, word, or
> long. If Rn is a
>
> data register and the operation size is byte or word, only the
> appropriate low-order part
>
> of Rn is checked. If Rn is an address register and the operation size is
> byte or word,
>
> the bounds operands are sign-extended to 32 bits, and the resultant
> operands are
>
> compared to the full 32 bits of An.
>
> If the upper bound equals the lower bound, the valid range is a single
> value.
>
>
>
> signed-off-by: Guolei <guol-fnst@cn.fujitsu.com
> <mailto:guol-fnst@cn.fujitsu.com%20>>
>
>
>
> ---
Please talk to your Fujitsu colleagues for how to correctly submit
patches to our mailing list. As you can see above, it arrives rather
"funny" due to your use of HTML. As a sanity check, you can mail the
patch to yourself, save it from your inbox to disk and try to apply it
via "git am yourmailfilename".
Why do you write "Description:" in the patch description? Is that a
verbatim copy from the manual? In that case you should name the source.
Otherwise please drop "Description:" and just describe it.
Also, Signed-off-by with a capital S please, and no space after your
email. If you use git commit -s you can avoid such issues from the start.
Please also use a meaningful topic in the subject, "target-m68k: Add
CMP2 instruction" tells us that this is not for x86 or ARM etc.
More information here:
http://wiki.qemu-project.org/Contribute/SubmitAPatch
As for the contents, you should be aware that QEMU currently emulates
ColdFire only and not the full 68k instruction set. So you may need to
limit your new instruction to certain CPU types. CC'ing Laurent+Andreas.
You haven't replied to Thomas' question on your previous submission
either. If you resubmit, please either mark as [PATCH RESEND], or if
things changed (such as adding the s ;)) as [PATCH v2] and include a
summary of changes below ---.
Note that there is currently no dedicated maintainer to review m68k
patches, so only obviously correct bug fixes are likely to get accepted
unless someone (you?) steps up for reviewing, testing and queuing such
patches on a Git branch.
Finally a minor nit: Please add a space between ) and {.
scripts/checkpatch.pl may help you find such style issues.
http://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html
Looking forward to seeing the m68k target improved.
Regards,
Andreas
--
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 21284 AG Nürnberg
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Add CMP2 instruction
2014-11-07 10:14 [Qemu-devel] Add CMP2 instruction Guo, Lei
2014-11-07 11:10 ` Alex Bennée
2014-11-07 11:15 ` Andreas Färber
@ 2014-11-07 11:26 ` Richard Henderson
2014-11-07 11:57 ` Laurent Vivier
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Richard Henderson @ 2014-11-07 11:26 UTC (permalink / raw)
To: Guo, Lei, qemu-devel@nongnu.org; +Cc: Wang, Chunping
On 11/07/2014 11:14 AM, Guo, Lei wrote:
> This patch aims to add CMP2 instruction for m68k family.
Mainline target-m68k supports coldfire only.
There is an external tree for full m68k support:
https://gitorious.org/qemu-m68k
That said, before you send this to them...
> + if (ext & 0x8000) {
> + reg = AREG(ext, 12);
Failure to sign-extend for opsize == OS_WORD.
You need to use signed comparisons for this case.
> + } else {
> + reg = DREG(ext, 12);
> + if (opsize == OS_BYTE){
> + tcg_gen_andi_i32(reg, reg, 0xf);
> + } else if (opsize == OS_WORD) {
> + tcg_gen_andi_i32(reg, reg, 0xff);
> + }
> + }
Incorrect zero-extension; you've messed up the constants.
Use tcg_gen_ext{8,16}u_i32, anyway.
You need to use unsigned comparisons for this case.
> + l1 = gen_new_label();
> + l2 = gen_new_label();
> + l3 = gen_new_label();
> + l4 = gen_new_label();
> +
> + tcg_gen_brcond_i32(TCG_COND_NE, reg, lower, l1);
Ew. You'd be much better off doing this with setcond than brcond.
gen_flush_flags(s);
t1 = tcg_temp_new();
t2 = tcg_temp_new();
t3 = tcg_temp_new();
t4 = tcg_temp_new();
tcg_gen_setcond_i32(TCG_COND_EQ, t1, reg, upper);
tcg_gen_setcond_i32(TCG_COND_EQ, t2, reg, lower);
tcg_gen_setcond_i32(which_gt, t3, reg, upper);
tcg_gen_setcond_i32(which_lt, t4, reg, lower);
tcg_gen_or_i32(t1, t1, t2); /* equal to either bound */
tcg_gen_or_i32(t3, t3, t4); /* out of bounds */
tcg_gen_shl_i32(t1, t1, ctz32(CCF_Z)); /* shift Z into place */
tcg_gen_shl_i32(t3, t3, ctz32(CCF_C)); /* shift C into place */
tcg_gen_or_i32(t1, t1, t3);
tcg_gen_andi_i32(QREG_CC_DEST, QREG_CC_DEST, ~(CCF_C | CCF_Z));
tcg_gen_or_i32(QREG_CC_DEST, QREG_CC_DEST, t1);
r~
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Add CMP2 instruction
2014-11-07 10:14 [Qemu-devel] Add CMP2 instruction Guo, Lei
` (2 preceding siblings ...)
2014-11-07 11:26 ` [Qemu-devel] " Richard Henderson
@ 2014-11-07 11:57 ` Laurent Vivier
2014-11-07 13:44 ` Laurent Vivier
2014-11-07 20:05 ` Andreas Schwab
5 siblings, 0 replies; 10+ messages in thread
From: Laurent Vivier @ 2014-11-07 11:57 UTC (permalink / raw)
To: qemu-devel@nongnu.org, Guo, Lei; +Cc: Wang, Chunping
[-- Attachment #1: Type: text/plain, Size: 5050 bytes --]
Hi,
did you test it ?
because after just a first glance this patch seems wrong as the tmp variables
cannot be used beyond the first tcg_gen_cond() (conditional branches clobber
temporary vars), you must use tcg_temp_local_new() and tcg_temp_free().
Regards,
Laurent
> Le 7 novembre 2014 à 11:14, "Guo, Lei" <guol-fnst@cn.fujitsu.com> a écrit :
>
>
> This patch aims to add CMP2 instruction for m68k family.
>
>
>
> Description: Compares the value in Rn to each bound. The effective address
> contains the
>
> bounds pair: upper bound following the lower bound. For signed comparisons,
> the
>
> arithmetically smaller value should be used as the lower bound. For unsigned
>
> comparisons, the logically smaller value should be the lower bound.
>
> The size of the data and the bounds can be specified as byte, word, or long.
> If Rn is a
>
> data register and the operation size is byte or word, only the appropriate
> low-order part
>
> of Rn is checked. If Rn is an address register and the operation size is byte
> or word,
>
> the bounds operands are sign-extended to 32 bits, and the resultant operands
> are
>
> compared to the full 32 bits of An.
>
> If the upper bound equals the lower bound, the valid range is a single value.
>
>
>
> signed-off-by: Guolei <guol-fnst@cn.fujitsu.com
> <mailto:guol-fnst@cn.fujitsu.com%20> >
>
>
>
> ---
>
> target-m68k/translate.c | 74
> +++++++++++++++++++++++++++++++++++++++++++++++
>
> 1 file changed, 74 insertions(+)
>
>
>
> diff --git a/target-m68k/translate.c b/target-m68k/translate.c
>
> index efd4cfc..8c09248 100644
>
> --- a/target-m68k/translate.c
>
> +++ b/target-m68k/translate.c
>
> @@ -1931,6 +1931,79 @@ DISAS_INSN(ff1)
>
> gen_helper_ff1(reg, reg);
>
> }
>
>
>
> +DISAS_INSN(cmp2)
>
> +{
>
> + TCGv src;
>
> + TCGv reg;
>
> + int opsize;
>
> + int l1, l2, l3, l4;
>
> + TCGv lower, upper;
>
> + uint16_t ext;
>
> +
>
> + switch ((insn >> 9) & 3) {
>
> + case 0:
>
> + opsize = OS_BYTE;
>
> + break;
>
> + case 1:
>
> + opsize = OS_WORD;
>
> + break;
>
> + case 2:
>
> + opsize = OS_LONG;
>
> + break;
>
> + default:
>
> + gen_exception(s, s->pc, EXCP_ILLEGAL);
>
> + return;
>
> + }
>
> +
>
> + SRC_EA(env, src, opsize, -1, NULL);
>
> + lower = tcg_temp_new();
>
> + upper = tcg_temp_new();
>
> + tcg_gen_shri_i32(lower, lower, 16);
>
> + tcg_gen_andi_i32(lower, lower, 0xffff);
>
> + tcg_gen_andi_i32(upper, upper, 0xffff);
>
> +
>
> + ext = cpu_ldsw_code(env, s->pc);
>
> + s->pc += 2;
>
> + if (ext & 0x8000) {
>
> + reg = AREG(ext, 12);
>
> + } else {
>
> + reg = DREG(ext, 12);
>
> + if (opsize == OS_BYTE){
>
> + tcg_gen_andi_i32(reg, reg, 0xf);
>
> + } else if (opsize == OS_WORD) {
>
> + tcg_gen_andi_i32(reg, reg, 0xff);
>
> + }
>
> + }
>
> +
>
> + l1 = gen_new_label();
>
> + l2 = gen_new_label();
>
> + l3 = gen_new_label();
>
> + l4 = gen_new_label();
>
> +
>
> + tcg_gen_brcond_i32(TCG_COND_NE, reg, lower, l1);
>
> + s->cc_op = CC_OP_FLAGS;
>
> + tcg_gen_ori_i32(QREG_CC_DEST, QREG_CC_DEST, CCF_Z);
>
> + tcg_gen_br(l4);
>
> +
>
> + gen_set_label(l1);
>
> + tcg_gen_brcond_i32(TCG_COND_NE, reg, upper, l2);
>
> + s->cc_op = CC_OP_FLAGS;
>
> + tcg_gen_ori_i32(QREG_CC_DEST, QREG_CC_DEST, CCF_Z);
>
> + tcg_gen_br(l4);
>
> +
>
> + gen_set_label(l2);
>
> + tcg_gen_brcond_i32(TCG_COND_GT, reg, lower, l3);
>
> + s->cc_op = CC_OP_FLAGS;
>
> + tcg_gen_ori_i32(QREG_CC_DEST, QREG_CC_DEST, CCF_C);
>
> + tcg_gen_br(l4);
>
> +
>
> + gen_set_label(l3);
>
> + tcg_gen_brcond_i32(TCG_COND_LT, reg, upper, l4);
>
> + s->cc_op = CC_OP_FLAGS;
>
> + tcg_gen_ori_i32(QREG_CC_DEST, QREG_CC_DEST, CCF_C);
>
> + gen_set_label(l4);
>
> +}
>
> +
>
> static TCGv gen_get_sr(DisasContext *s)
>
> {
>
> TCGv ccr;
>
> @@ -2847,6 +2920,7 @@ void register_m68k_insns (CPUM68KState *env)
>
> INSN(bitop_reg, 01c0, f1c0, CF_ISA_A);
>
> INSN(arith_im, 0280, fff8, CF_ISA_A);
>
> INSN(byterev, 02c0, fff8, CF_ISA_APLUSC);
>
> + INSN(cmp2, 02c0, ffff, CF_ISA_A);
>
> INSN(arith_im, 0480, fff8, CF_ISA_A);
>
> INSN(ff1, 04c0, fff8, CF_ISA_APLUSC);
>
> INSN(arith_im, 0680, fff8, CF_ISA_A);
>
> --
>
> 1.7.9.5
>
[-- Attachment #2: Type: text/html, Size: 13794 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Add CMP2 instruction
2014-11-07 10:14 [Qemu-devel] Add CMP2 instruction Guo, Lei
` (3 preceding siblings ...)
2014-11-07 11:57 ` Laurent Vivier
@ 2014-11-07 13:44 ` Laurent Vivier
2014-11-07 20:05 ` Andreas Schwab
5 siblings, 0 replies; 10+ messages in thread
From: Laurent Vivier @ 2014-11-07 13:44 UTC (permalink / raw)
To: qemu-devel@nongnu.org, Guo, Lei; +Cc: Wang, Chunping
> Le 7 novembre 2014 à 11:14, "Guo, Lei" <guol-fnst@cn.fujitsu.com> a écrit :
>
>
> This patch aims to add CMP2 instruction for m68k family.
>
>
>
> Description: Compares the value in Rn to each bound. The effective address
> contains the
>
> bounds pair: upper bound following the lower bound. For signed comparisons,
> the
>
> arithmetically smaller value should be used as the lower bound. For unsigned
>
> comparisons, the logically smaller value should be the lower bound.
>
> The size of the data and the bounds can be specified as byte, word, or long.
> If Rn is a
>
> data register and the operation size is byte or word, only the appropriate
> low-order part
>
> of Rn is checked. If Rn is an address register and the operation size is byte
> or word,
>
> the bounds operands are sign-extended to 32 bits, and the resultant operands
> are
>
> compared to the full 32 bits of An.
>
> If the upper bound equals the lower bound, the valid range is a single value.
>
Moreover, this seems a cut&paste of the Motorola "Programmer's reference
manual".
It should be copyrighted : have we the right to copy this part in a commit
message ?
Regards,
Laurent
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Add CMP2 instruction
2014-11-07 10:14 [Qemu-devel] Add CMP2 instruction Guo, Lei
` (4 preceding siblings ...)
2014-11-07 13:44 ` Laurent Vivier
@ 2014-11-07 20:05 ` Andreas Schwab
5 siblings, 0 replies; 10+ messages in thread
From: Andreas Schwab @ 2014-11-07 20:05 UTC (permalink / raw)
To: Guo, Lei; +Cc: qemu-devel@nongnu.org, Wang, Chunping
"Guo, Lei" <guol-fnst@cn.fujitsu.com> writes:
> @@ -2847,6 +2920,7 @@ void register_m68k_insns (CPUM68KState *env)
> INSN(bitop_reg, 01c0, f1c0, CF_ISA_A);
> INSN(arith_im, 0280, fff8, CF_ISA_A);
> INSN(byterev, 02c0, fff8, CF_ISA_APLUSC);
> + INSN(cmp2, 02c0, ffff, CF_ISA_A);
My copy of the CFPRM doesn't list cmp2 as a valid coldfire insn.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Add CMP2 instruction
@ 2014-11-10 1:35 Guo, Lei
0 siblings, 0 replies; 10+ messages in thread
From: Guo, Lei @ 2014-11-10 1:35 UTC (permalink / raw)
To: Laurent Vivier; +Cc: 'qemu-devel@nongnu.org', Wang, Chunping
Hi Laurent
This pagraph exactly is a cut&paste of the Motorola "Programmer's reference manual".
I didn't mean to add it as commits to the patch.I just want to introduce this instruction to the reviewer.
So don't worry about the copyrights.
Best Regards
Guo Lei
>
>
>
> > Le 7 novembre 2014 à 11:14, "Guo, Lei" <guol-fnst@cn.fujitsu.com> a écrit :
> >
> >
> > This patch aims to add CMP2 instruction for m68k family.
> >
> >
> >
> > Description: Compares the value in Rn to each bound. The effective
> > address contains the
> >
> > bounds pair: upper bound following the lower bound. For signed
> > comparisons, the
> >
> > arithmetically smaller value should be used as the lower bound. For
> > unsigned
> >
> > comparisons, the logically smaller value should be the lower bound.
> >
> > The size of the data and the bounds can be specified as byte, word, or long.
> > If Rn is a
> >
> > data register and the operation size is byte or word, only the
> > appropriate low-order part
> >
> > of Rn is checked. If Rn is an address register and the operation
> > size is byte or word,
> >
> > the bounds operands are sign-extended to 32 bits, and the resultant
> > operands are
> >
> > compared to the full 32 bits of An.
> >
> > If the upper bound equals the lower bound, the valid range is a single value.
> >
>
> Moreover, this seems a cut&paste of the Motorola "Programmer's
> reference manual".
> It should be copyrighted : have we the right to copy this part in a
> commit message ?
>
> Regards,
> Laurent
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] 答复: Add CMP2 instruction
2014-11-07 11:15 ` Andreas Färber
@ 2014-11-10 3:21 ` Guo, Lei
2014-11-10 11:30 ` Laurent Vivier
0 siblings, 1 reply; 10+ messages in thread
From: Guo, Lei @ 2014-11-10 3:21 UTC (permalink / raw)
To: Andreas Färber, qemu-devel@nongnu.org
Cc: Andreas Schwab, Laurent Vivier, Thomas Huth
Hi Andreas
Thanks a lot for your patients. Because I'm a newer to this , I'll follow your advices and pay much more attention to these details.
Besides ,I have replied to Thomas' question on my previous submission.
Thanks again for your help.
Best regards
Guo lei
Re: [Qemu-devel] Add CMP2 instruction
>
> Hi,
>
> Am 07.11.2014 um 11:14 schrieb Guo, Lei:
> > This patch aims to add CMP2 instruction for m68k family.
> >
> >
> >
> > *Description: *Compares the value in Rn to each bound. The effective
> > address contains the
> >
> > bounds pair: upper bound following the lower bound. For signed
> > comparisons, the
> >
> > arithmetically smaller value should be used as the lower bound. For
> > unsigned
> >
> > comparisons, the logically smaller value should be the lower bound.
> >
> > The size of the data and the bounds can be specified as byte, word, or
> > long. If Rn is a
> >
> > data register and the operation size is byte or word, only the
> > appropriate low-order part
> >
> > of Rn is checked. If Rn is an address register and the operation size
> > is byte or word,
> >
> > the bounds operands are sign-extended to 32 bits, and the resultant
> > operands are
> >
> > compared to the full 32 bits of An.
> >
> > If the upper bound equals the lower bound, the valid range is a single
> > value.
> >
> >
> >
> > signed-off-by: Guolei <guol-fnst@cn.fujitsu.com
> > <mailto:guol-fnst@cn.fujitsu.com%20>>
> >
> >
> >
> > ---
>
> Please talk to your Fujitsu colleagues for how to correctly submit patches to our
> mailing list. As you can see above, it arrives rather "funny" due to your use of
> HTML. As a sanity check, you can mail the patch to yourself, save it from your
> inbox to disk and try to apply it via "git am yourmailfilename".
>
> Why do you write "Description:" in the patch description? Is that a verbatim
> copy from the manual? In that case you should name the source.
> Otherwise please drop "Description:" and just describe it.
>
> Also, Signed-off-by with a capital S please, and no space after your email. If you
> use git commit -s you can avoid such issues from the start.
>
> Please also use a meaningful topic in the subject, "target-m68k: Add
> CMP2 instruction" tells us that this is not for x86 or ARM etc.
>
> More information here:
> http://wiki.qemu-project.org/Contribute/SubmitAPatch
>
> As for the contents, you should be aware that QEMU currently emulates
> ColdFire only and not the full 68k instruction set. So you may need to limit your
> new instruction to certain CPU types. CC'ing Laurent+Andreas.
>
> You haven't replied to Thomas' question on your previous submission either. If
> you resubmit, please either mark as [PATCH RESEND], or if things changed
> (such as adding the s ;)) as [PATCH v2] and include a summary of changes below
> ---.
>
> Note that there is currently no dedicated maintainer to review m68k patches,
> so only obviously correct bug fixes are likely to get accepted unless someone
> (you?) steps up for reviewing, testing and queuing such patches on a Git
> branch.
>
> Finally a minor nit: Please add a space between ) and {.
> scripts/checkpatch.pl may help you find such style issues.
> http://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html
>
> Looking forward to seeing the m68k target improved.
>
> Regards,
> Andreas
>
> --
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 21284 AG Nürnberg
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] 答复: Add CMP2 instruction
2014-11-10 3:21 ` [Qemu-devel] 答复: " Guo, Lei
@ 2014-11-10 11:30 ` Laurent Vivier
0 siblings, 0 replies; 10+ messages in thread
From: Laurent Vivier @ 2014-11-10 11:30 UTC (permalink / raw)
To: Guo, Lei, Andreas Färber, qemu-devel@nongnu.org
Cc: Andreas Schwab, Thomas Huth
[-- Attachment #1: Type: text/plain, Size: 1044 bytes --]
Le 10/11/2014 04:21, Guo, Lei a écrit :
> Hi Andreas
> Thanks a lot for your patients. Because I'm a newer to this , I'll follow your advices and pay much more attention to these details.
> Besides ,I have replied to Thomas' question on my previous submission.
If the question was "Have you been in touch with the author of that
original patch already?", I can affirm the answer is "no".
It's opensoure, grab what you want, but if you resend a patch you must
let the "Signed-off-by".
BTW, "CMP2" is neither generated by gcc or used by linux, why do you
want to add it to an old and obsolete architecture ? Do you have a "real
life" test case for it ?
If you send a correct and tested new patch, I can add it in my tree.
Mainline QEMU doesn't care of m68k (680x0) support.
For those who want to know why I don't submit my patches the reasons are :
- they break coldfire support
- there is a bug in MMU emulation that prevent linux to fork new
processes (I'm hunting this for 2 years now...)
Regards,
Laurent
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-11-10 11:31 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-07 10:14 [Qemu-devel] Add CMP2 instruction Guo, Lei
2014-11-07 11:10 ` Alex Bennée
2014-11-07 11:15 ` Andreas Färber
2014-11-10 3:21 ` [Qemu-devel] 答复: " Guo, Lei
2014-11-10 11:30 ` Laurent Vivier
2014-11-07 11:26 ` [Qemu-devel] " Richard Henderson
2014-11-07 11:57 ` Laurent Vivier
2014-11-07 13:44 ` Laurent Vivier
2014-11-07 20:05 ` Andreas Schwab
-- strict thread matches above, loose matches on Subject: below --
2014-11-10 1:35 Guo, Lei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).