Re: [PATCH 1/2]middle-end: Add new tbranch optab to add support for bit-test-and-branch operations

[Jeff Law <jeffreyalaw@gmail.com>]

On 10/31/22 05:53, Tamar Christina wrote:
> Hi All,
>
> This adds a new test-and-branch optab that can be used to do a conditional test
> of a bit and branch.   This is similar to the cbranch optab but instead can
> test any arbitrary bit inside the register.
>
> This patch recognizes boolean comparisons and single bit mask tests.
>
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>
> Ok for master?
>
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
> 	* dojump.cc (do_jump): Pass along value.
> 	(do_jump_by_parts_greater_rtx): Likewise.
> 	(do_jump_by_parts_zero_rtx): Likewise.
> 	(do_jump_by_parts_equality_rtx): Likewise.
> 	(do_compare_rtx_and_jump): Likewise.
> 	(do_compare_and_jump): Likewise.
> 	* dojump.h (do_compare_rtx_and_jump): New.
> 	* optabs.cc (emit_cmp_and_jump_insn_1): Refactor to take optab to check.
> 	(validate_test_and_branch): New.
> 	(emit_cmp_and_jump_insns): Optiobally take a value, and when value is
> 	supplied then check if it's suitable for tbranch.
> 	* optabs.def (tbranch$a4): New.
> 	* doc/md.texi (tbranch@var{mode}4): Document it.
> 	* optabs.h (emit_cmp_and_jump_insns):
> 	* tree.h (tree_zero_one_valued_p): New.
>
> --- inline copy of patch --
> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> index c08691ab4c9a4bfe55ae81e5e228a414d6242d78..f8b32ec12f46d3fb3815f121a16b5a8a1819b66a 100644
> --- a/gcc/doc/md.texi
> +++ b/gcc/doc/md.texi
> @@ -6972,6 +6972,13 @@ case, you can and should make operand 1's predicate reject some operators
>   in the @samp{cstore@var{mode}4} pattern, or remove the pattern altogether
>   from the machine description.
>   
> +@cindex @code{tbranch@var{mode}4} instruction pattern
> +@item @samp{tbranch@var{mode}4}
> +Conditional branch instruction combined with a bit test-and-compare
> +instruction. Operand 0 is a comparison operator.  Operand 1 is the
> +operand of the comparison. Operand 2 is the bit position of Operand 1 to test.
> +Operand 3 is the @code{code_label} to jump to.

Should we refine/document the set of comparison operators allowed?    Is 
operand 1 an arbitrary RTL expression or more limited?  I'm guessing its 
relatively arbitrary given how you've massaged the existing 
branch-on-bit patterns from the aarch backend.


> +
> +  if (TREE_CODE (val) != SSA_NAME)
> +    return false;
> +
> +  gimple *def = SSA_NAME_DEF_STMT (val);
> +  if (!is_gimple_assign (def)
> +      || gimple_assign_rhs_code (def) != BIT_AND_EXPR)
> +    return false;
> +
> +  tree cst = gimple_assign_rhs2 (def);
> +
> +  if (!tree_fits_uhwi_p (cst))
> +    return false;
> +
> +  tree op0 = gimple_assign_rhs1 (def);
> +  if (TREE_CODE (op0) == SSA_NAME)
> +    {
> +      def = SSA_NAME_DEF_STMT (op0);
> +      if (gimple_assign_cast_p (def))
> +	op0 = gimple_assign_rhs1 (def);
> +    }
> +
> +  wide_int wcst = wi::uhwi (tree_to_uhwi (cst),
> +			    TYPE_PRECISION (TREE_TYPE (op0)));
> +  int bitpos;
> +
> +  if ((bitpos = wi::exact_log2 (wcst)) == -1)
> +    return false;

Do we have enough information lying around from Ranger to avoid the need 
to walk the def-use chain to discover that we're masking off all but one 
bit?



>   
>
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 8f8a9660c9e0605eb516de194640b8c1b531b798..be3d2dee82f692e81082cf21c878c10f9fe9e1f1 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -4690,6 +4690,7 @@ extern tree signed_or_unsigned_type_for (int, tree);
>   extern tree signed_type_for (tree);
>   extern tree unsigned_type_for (tree);
>   extern bool is_truth_type_for (tree, tree);
> +extern bool tree_zero_one_valued_p (tree);

I don't see a definition of this anywhere.


jeff