Re: [PULL v2 00/35] hexagon initial commit

[Richard Henderson]

On 2/19/21 8:30 AM, Taylor Simpson wrote:
> I requested access to scan.coverity.com.  Once it is granted, I'll take a 
> look.

I took a quick look.  Quite a lot of the errors are related to

> #define fASHIFTL(SRC, SHAMT, REGSTYPE) \
>     (((SHAMT) >= 64) ? 0 : (fCAST##REGSTYPE##s(SRC) << (SHAMT)))

and

> #define fLSHIFTR(SRC, SHAMT, REGSTYPE) \
>     (((SHAMT) >= 64) ? 0 : (fCAST##REGSTYPE##u(SRC) >> (SHAMT)))

Coverity does not look beyond the leading comparison to inform the bounds, and
these macros are used with a 32-bit type.  It then warns that the shift could
be out of bounds.

It appears that none of the uses of fASHIFTL can actually overflow the shift 
count:

  * S2_asl_i has a 5-bit immediate shift count.
  * S2_addasl_rrri has a 3-bit immediate shift count.
  * S2_valign has a 3-bit scaled immediate shift count
    (on a 64-bit type).

So it looks like you could simply drop the tests entirely.
If you really want to keep it, then you should make use of REGSTYPE and bound
based on that.


r~