On 07/16/2018 10:10 AM, Sam Tebbs wrote:
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -1439,6 +1439,14 @@ aarch64_hard_regno_caller_save_mode (unsigned regno,
> unsigned,
> return SImode;
> }
>
> +/* Implement IS_LEFT_CONSECUTIVE. Check if an integer's bits are consecutive
> + ones from the MSB. */
> +bool
> +aarch64_is_left_consecutive (HOST_WIDE_INT i)
> +{
> + return (i | (i - 1)) == HOST_WIDE_INT_M1;
> +}
> +
...
> +(define_insn "*aarch64_bfxil"
> + [(set (match_operand:DI 0 "register_operand" "=r")
> + (ior:DI (and:DI (match_operand:DI 1 "register_operand" "r")
> + (match_operand 3 "const_int_operand"))
> + (and:DI (match_operand:DI 2 "register_operand" "0")
> + (match_operand 4 "const_int_operand"))))]
> + "INTVAL (operands[3]) == ~INTVAL (operands[4])
> + && aarch64_is_left_consecutive (INTVAL (operands[4]))"
Better is to use a define_predicate to merge both that second test and the
const_int_operand.
(I'm not sure about the "left_consecutive" language either.
Isn't it more descriptive to say that op3 is a power of 2 minus 1?)
(define_predicate "pow2m1_operand"
(and (match_code "const_int")
(match_test "exact_pow2 (INTVAL(op) + 1) > 0")))
and use
(match_operand:DI 3 "pow2m1_operand")
and then just the
INTVAL (operands[3]) == ~INTVAL (operands[4])
test.
Also, don't omit the modes for the constants.
Also, there's no reason this applies only to DI mode;
use the GPI iterator and %<w> in the output template.
> + HOST_WIDE_INT op3 = INTVAL (operands[3]);
> + operands[3] = GEN_INT (ceil_log2 (op3));
> + output_asm_insn ("bfxil\\t%0, %1, 0, %3", operands);
> + return "";
You can just return the string that you passed to output_asm_insn.
> + }
> + [(set_attr "type" "bfx")]
The other aliases of the BFM insn use type "bfm";
"bfx" appears to be aliases of UBFM and SBFM.
Not that it appears to matter to the scheduling
descriptions, but it is inconsistent.
r~
