On Wed, Nov 13, 2024 at 09:22:45AM +0100, Richard Biener wrote:
> While I'm far from an expert here this doesn't look right and instead the
> const_0_to_255_operand looks bogus to me in not properly taking into
> account 'mode'.
I think the bug is in use of const_0_to_255* predicates with QImode or
mode-less operands, i.e.
grep const_0_to_255 *.md | grep -v ':SI\|define_predicate'
i386.md: (match_operand:QI 3 "const_0_to_255_operand")) 0)))]
i386.md: (match_operand:QI 4 "const_0_to_255_operand")) 0)))]
i386.md: (match_operand:QI 3 "const_0_to_255_operand")) 0)))]
i386.md: (match_operand:QI 4 "const_0_to_255_operand")) 0)))]
i386.md: (match_operand:QI 2 "const_0_to_255_operand")
i386.md: (match_operand:QI 3 "const_0_to_255_operand")))
sse.md: (match_operand 2 "const_0_to_255_operand")))
sse.md: (match_operand 2 "const_0_to_255_operand")
sse.md: (match_operand 3 "const_0_to_255_operand")]
sse.md: (match_operand 3 "const_0_to_255_operand")
sse.md: (match_operand 4 "const_0_to_255_operand")]
sse.md: (match_operand 2 "const_0_to_255_operand")]
sse.md: (match_operand 2 "const_0_to_255_operand")]
sse.md: (match_operand 3 "const_0_to_255_operand")]
sse.md: (match_operand 3 "const_0_to_255_operand")]
sse.md: (match_operand 2 "const_0_to_255_operand")]
With QImode, const_0_to_255_operand makes no sense, valid values are
-128..127. So, if QImode in that case is right, just use const_int_operand.
And obviously the INTVAL uses on that operand need to be prepared for the
value being -128..127. I believe during expansion if the match_operand is
QImode that it will be turned into -128..127.
The VOIDmode ones even worse because I think nothing will truncate the
constants in there. So, either turn those into QImode with
const_int_operand or SImode with const_0_to_255_operand, but in the latter
case change also i386-builtin.def to pass in SImode (UINT) operand rather than
QImode.
Basically, with QImode there can't be any in-range checking for error
diagnostics, all values are valid. If we want checking, the builtin
should have SImode or similar argument (or whatever the intrinsic argument
type is) and so should be the match_operand.
E.g.
#include <x86intrin.h>
__mmask64 foo (__mmask64 x) { return _kshiftli_mask64 (x, 257); }
errors with clang/icx, warns with icc but is silently accepted by gcc.
That looks wrong to me.
extern __inline __mmask64
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
_kshiftli_mask64 (__mmask64 __A, unsigned int __B)
{
return (__mmask64) __builtin_ia32_kshiftlidi ((__mmask64) __A,
(__mmask8) __B);
}
The second argument is unsigned int shift count, so shouldn't be cast
to __mmask8 which is for masks, you don't shift a mask by mask but mask by
unsigned integer.
Jakub
