| Message ID | 20170724182751.18261-24-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
Le 24/07/2017 à 20:27, Philippe Mathieu-Daudé a écrit : > db3d7945ae extended gen_cc_cond() for cond [6, 7, 9, 10] but misswrote [4, 5] > > target/m68k/translate.c:1323:70: warning: identical expressions on both sides of logical operator > if (op == CC_OP_ADDB || op == CC_OP_ADDW || op == CC_OP_ADDL || > op == CC_OP_ADDB || op == CC_OP_ADDW || op == CC_OP_ADDL) { > ~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~ ^ > > Reported-by: Clang Static Analyzer > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > target/m68k/translate.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/target/m68k/translate.c b/target/m68k/translate.c > index ada2a91b64..1a2f421aab 100644 > --- a/target/m68k/translate.c > +++ b/target/m68k/translate.c > @@ -1321,7 +1321,8 @@ static void gen_cc_cond(DisasCompare *c, DisasContext *s, int cond) > case 5: /* CS (C) */ > /* Some cases fold C into X. */ > if (op == CC_OP_ADDB || op == CC_OP_ADDW || op == CC_OP_ADDL || > - op == CC_OP_ADDB || op == CC_OP_ADDW || op == CC_OP_ADDL) { > + op == CC_OP_SUBB || op == CC_OP_SUBW || op == CC_OP_SUBL || > + op == CC_OP_LOGIC) { According to commit (db3d7945) introducing the incorrect copy/paste, I don't think we need the CC_OP_LOGIC here. Logic operations never generates X flags (whereas they can generate Z and and N). Thanks, Laurent
On 07/24/2017 11:54 AM, Laurent Vivier wrote: > Le 24/07/2017 à 20:27, Philippe Mathieu-Daudé a écrit : >> diff --git a/target/m68k/translate.c b/target/m68k/translate.c >> index ada2a91b64..1a2f421aab 100644 >> --- a/target/m68k/translate.c >> +++ b/target/m68k/translate.c >> @@ -1321,7 +1321,8 @@ static void gen_cc_cond(DisasCompare *c, DisasContext *s, int cond) >> case 5: /* CS (C) */ >> /* Some cases fold C into X. */ >> if (op == CC_OP_ADDB || op == CC_OP_ADDW || op == CC_OP_ADDL || >> - op == CC_OP_ADDB || op == CC_OP_ADDW || op == CC_OP_ADDL) { >> + op == CC_OP_SUBB || op == CC_OP_SUBW || op == CC_OP_SUBL || >> + op == CC_OP_LOGIC) { > > According to commit (db3d7945) introducing the incorrect copy/paste, I > don't think we need the CC_OP_LOGIC here. Logic operations never > generates X flags (whereas they can generate Z and and N). Indeed, look at the comment below. LOGIC is supposed to fall through to the code that produces TCG_COND_NEVER. r~
On 07/24/2017 04:01 PM, Richard Henderson wrote: > On 07/24/2017 11:54 AM, Laurent Vivier wrote: >> Le 24/07/2017 à 20:27, Philippe Mathieu-Daudé a écrit : >>> diff --git a/target/m68k/translate.c b/target/m68k/translate.c >>> index ada2a91b64..1a2f421aab 100644 >>> --- a/target/m68k/translate.c >>> +++ b/target/m68k/translate.c >>> @@ -1321,7 +1321,8 @@ static void gen_cc_cond(DisasCompare *c, >>> DisasContext *s, int cond) >>> case 5: /* CS (C) */ >>> /* Some cases fold C into X. */ >>> if (op == CC_OP_ADDB || op == CC_OP_ADDW || op == >>> CC_OP_ADDL || >>> - op == CC_OP_ADDB || op == CC_OP_ADDW || op == CC_OP_ADDL) { >>> + op == CC_OP_SUBB || op == CC_OP_SUBW || op == CC_OP_SUBL || >>> + op == CC_OP_LOGIC) { >> >> According to commit (db3d7945) introducing the incorrect copy/paste, I >> don't think we need the CC_OP_LOGIC here. Logic operations never >> generates X flags (whereas they can generate Z and and N). > > Indeed, look at the comment below. LOGIC is supposed to fall through to > the code that produces TCG_COND_NEVER. Indeed I also miss-copypasted :) So the correct fix would be:? if (op == CC_OP_ADDB || op == CC_OP_ADDW || op == CC_OP_ADDL || - op == CC_OP_ADDB || op == CC_OP_ADDW || op == CC_OP_ADDL) { + op == CC_OP_SUBB || op == CC_OP_SUBW || op == CC_OP_SUBL) {
Le 24/07/2017 à 21:19, Philippe Mathieu-Daudé a écrit : > On 07/24/2017 04:01 PM, Richard Henderson wrote: >> On 07/24/2017 11:54 AM, Laurent Vivier wrote: >>> Le 24/07/2017 à 20:27, Philippe Mathieu-Daudé a écrit : >>>> diff --git a/target/m68k/translate.c b/target/m68k/translate.c >>>> index ada2a91b64..1a2f421aab 100644 >>>> --- a/target/m68k/translate.c >>>> +++ b/target/m68k/translate.c >>>> @@ -1321,7 +1321,8 @@ static void gen_cc_cond(DisasCompare *c, >>>> DisasContext *s, int cond) >>>> case 5: /* CS (C) */ >>>> /* Some cases fold C into X. */ >>>> if (op == CC_OP_ADDB || op == CC_OP_ADDW || op == >>>> CC_OP_ADDL || >>>> - op == CC_OP_ADDB || op == CC_OP_ADDW || op == >>>> CC_OP_ADDL) { >>>> + op == CC_OP_SUBB || op == CC_OP_SUBW || op == >>>> CC_OP_SUBL || >>>> + op == CC_OP_LOGIC) { >>> >>> According to commit (db3d7945) introducing the incorrect copy/paste, I >>> don't think we need the CC_OP_LOGIC here. Logic operations never >>> generates X flags (whereas they can generate Z and and N). >> >> Indeed, look at the comment below. LOGIC is supposed to fall through >> to the code that produces TCG_COND_NEVER. > > Indeed I also miss-copypasted :) > > So the correct fix would be:? > > if (op == CC_OP_ADDB || op == CC_OP_ADDW || op == CC_OP_ADDL || > - op == CC_OP_ADDB || op == CC_OP_ADDW || op == CC_OP_ADDL) { > + op == CC_OP_SUBB || op == CC_OP_SUBW || op == CC_OP_SUBL) { yes Reviewed-by: Laurent Vivier <laurent@vivier.eu> Laurent
diff --git a/target/m68k/translate.c b/target/m68k/translate.c index ada2a91b64..1a2f421aab 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -1321,7 +1321,8 @@ static void gen_cc_cond(DisasCompare *c, DisasContext *s, int cond) case 5: /* CS (C) */ /* Some cases fold C into X. */ if (op == CC_OP_ADDB || op == CC_OP_ADDW || op == CC_OP_ADDL || - op == CC_OP_ADDB || op == CC_OP_ADDW || op == CC_OP_ADDL) { + op == CC_OP_SUBB || op == CC_OP_SUBW || op == CC_OP_SUBL || + op == CC_OP_LOGIC) { tcond = TCG_COND_NE; c->v1 = QREG_CC_X; goto done;
db3d7945ae extended gen_cc_cond() for cond [6, 7, 9, 10] but misswrote [4, 5] target/m68k/translate.c:1323:70: warning: identical expressions on both sides of logical operator if (op == CC_OP_ADDB || op == CC_OP_ADDW || op == CC_OP_ADDL || op == CC_OP_ADDB || op == CC_OP_ADDW || op == CC_OP_ADDL) { ~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~ ^ Reported-by: Clang Static Analyzer Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- target/m68k/translate.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)