[GCC][PATCH][Aarch64] Exploiting BFXIL when OR-ing two AND-operations with appropriate bitmasks

[Richard Earnshaw (lists)]

On 17/07/18 02:33, Richard Henderson wrote:
> 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")))

ITYM exact_log2()

R.

> 
> 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~
>