On Mon, Dec 05, 2016 at 08:56:30PM -0700, Kelvin Nilsen wrote:
> +;; Predicate: test byte within range.
Just "Test byte within range."?
> +;; Return in target register operand 0 a value of 1 if the byte
> +;; held in bits 24:31 of operand 1 is within the inclusive range
> +;; bounded above by operand 2's bits 0:7 and below by operand 2's
> +;; bits 8:15. Otherwise, set register operand 0 to 0.
> +(define_expand "cmprb"
> + [(set (match_dup 3)
> + (unspec:CC [(match_operand:SI 1 "gpc_reg_operand" "r")
> + (match_operand:SI 2 "gpc_reg_operand" "r")]
> + UNSPEC_CMPRB))
> + (set (match_operand:SI 0 "gpc_reg_operand" "=r")
> + (if_then_else:SI (lt (match_dup 3)
> + (const_int 0))
> + (const_int -1)
> + (if_then_else (gt (match_dup 3)
> + (const_int 0))
> + (const_int 1)
> + (const_int 0))))]
> + "TARGET_P9_MISC"
> +{
> + operands[3] = gen_reg_rtx (CCmode);
> +})
Should this use the "GPR" iterator? Or you don't use DI ever yet...
But note it as a todo, maybe?
The range is not in bits 0..7 8..15, fwiw. It is probably less confusing
if you described it without bit numbers? Low bound in the lowest byte,
high bound in the byte just above. xx:xx:hi:lo
> +(define_expand "cmpeqb"
> + [(set (match_dup 3)
> + (unspec:CC [(match_operand:SI 1 "gpc_reg_operand" "r")
> + (match_operand:DI 2 "gpc_reg_operand" "r")]
> + UNSPEC_CMPEQB))
> + (set (match_operand:SI 0 "gpc_reg_operand" "=r")
> + (if_then_else:SI (lt (match_dup 3)
> + (const_int 0))
> + (const_int -1)
> + (if_then_else (gt (match_dup 3)
> + (const_int 0))
> + (const_int 1)
> + (const_int 0))))]
> + "TARGET_P9_MISC && TARGET_64BIT"
> +{
> + operands[3] = gen_reg_rtx (CCmode);
> +})
cmpeqb only ever sets GT, so the if_then_else is weird (but correct :-) )
Please fix.
> +#define BU_P9V_64BIT_AV_2(ENUM, NAME, ATTR, ICODE) \
> + RS6000_BUILTIN_2 (P9V_BUILTIN_ ## ENUM, /* ENUM */ \
> + "__builtin_altivec_" NAME, /* NAME */ \
> + RS6000_BTM_P9_VECTOR \
> + | RS6000_BTM_64BIT, /* MASK */ \
> + (RS6000_BTC_ ## ATTR /* ATTR */ \
> + | RS6000_BTC_BINARY), \
> + CODE_FOR_ ## ICODE) /* ICODE */
Strange that that builtin is called "..altivec..", is that correct?
> +/* 2 argument functions added in ISA 3.0 (power9). */
> +BU_P9V_AV_2 (CMPRB, "byte_in_range", CONST, cmprb)
> +BU_P9V_AV_2 (CMPRB2, "byte_in_either_range", CONST, cmprb2)
> +BU_P9V_64BIT_AV_2 (CMPEQB, "byte_in_set", CONST, cmpeqb)
> +
> +/* 2 argument overloaded functions added in ISA 3.0 (power9). */
> +BU_P9_OVERLOAD_2 (CMPRB, "byte_in_range")
> +BU_P9_OVERLOAD_2 (CMPRB2, "byte_in_either_range")
> +BU_P9_OVERLOAD_2 (CMPEQB, "byte_in_set")
> +int
> +test_byte_in_range (unsigned char b,
> + unsigned char low_range, unsigned char high_range)
> +{
> + unsigned int range_encoding = (high_range << 24) | (low_range << 16);
> + return __builtin_scalar_byte_in_range (b, range_encoding);
> +}
According to the ISA this should be in the *low* two bytes. So
+ unsigned int range_encoding = (high_range << 8) | low_range;
Or is the builtin different? That would be strange.
Thanks,
Segher
