On Tue, Dec 25, 2018 at 8:35 PM Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> Sorry I missed the original post, but:
>
> > + } else if (unlikely((XRb == 0) && (XRc == 0))) {
> > + /* both operands zero registers -> just set destination to zero */
> > + tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
> > + } else if (unlikely((XRb == 0) || (XRc == 0))) {
> > + /* exactly one operand is zero register - find which one is
> > not...*/
> > + uint32_t XRx = XRb ? XRb : XRc;
> > + /* ...and do max/min operation with one operand 0 */
> > + if (opc == OPC_MXU_S32MAX) {
> > + tcg_gen_smax_i32(mxu_gpr[XRa - 1], mxu_gpr[XRx - 1], 0);
> > + } else {
> > + tcg_gen_smin_i32(mxu_gpr[XRa - 1], mxu_gpr[XRx - 1], 0);
> > + }
> > + } else if (unlikely(XRb == XRc)) {
> > + /* both operands same -> just set destination to one of them */
> > + tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
>
> You should not special case unlikely events, especially when ...
>
> > + } else {
> > + /* the most general case */
> > + if (opc == OPC_MXU_S32MAX) {
> > + tcg_gen_smax_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1],
> > + mxu_gpr[XRc - 1]);
> > + } else {
> > + tcg_gen_smin_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1],
> > + mxu_gpr[XRc - 1]);
> > + }
>
> ... the normal case will handle those special cases just fine.
Also because we have now 3 Coverity CID:
*** CID 1450831: (OVERRUN)
in gen_mxu_D16MAX_D16MIN()
1107 TCGv_i32 t0 = tcg_temp_new();
1108 TCGv_i32 t1 = tcg_const_i32(0);
1109
1110 /* the left half-word first */
1111 tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0xFFFF0000);
1112 if (opc == OPC_MXU_D16MAX) {
>>> CID 1450831: (OVERRUN)
>>> Overrunning array "mxu_gpr" of 15 8-byte elements at element index
>>> 4294967295 (byte offset 34359738367) using index "XRa - 1U" (which
>>> evaluates to 4294967295).
1113 tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
>>> CID 1450831: (OVERRUN)
>>> Overrunning array "mxu_gpr" of 15 8-byte elements at element index
>>> 4294967295 (byte offset 34359738367) using index "XRa - 1U" (which
>>> evaluates to 4294967295).
1114 } else {
1115 tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
1116 }
1117
1118 /* the right half-word */
1119 tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x0000FFFF);
1125 } else {
1126 tcg_gen_smin_i32(t0, t0, t1);
1127 }
1128 /* return resulting half-words to its original position */
1129 tcg_gen_shri_i32(t0, t0, 16);
1130 /* finally update the destination */
>>> CID 1450831: (OVERRUN)
>>> Overrunning array "mxu_gpr" of 15 8-byte elements at element index
>>> 4294967295 (byte offset 34359738367) using index "XRa - 1U" (which
>>> evaluates to 4294967295).
1131 tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
1132
1133 tcg_temp_free(t1);
1134 tcg_temp_free(t0);
1135 } else if (unlikely(XRb == XRc)) {
1136 /* both operands same -> just set destination to one of them */
