From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id C39A13858C50 for ; Tue, 11 Jul 2023 11:03:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C39A13858C50 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 04A7C1FD70; Tue, 11 Jul 2023 11:03:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1689073403; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=J1Bb87U5x3S6hNg/bp/WXsFJEUGzju+SI4UnFd3g7VA=; b=Zey24t9lcYqY7Ro8+0y3sBcgwxPcIM8W0YBJbIOh0ma7ECNSyCQEoh25UgvkVGRn7z7uLa fkEIrc8GGZQKsfXx3WgISPP0nh6fqIc1PE68IuxfOGqL8PUN2zvKKuLznii0VxDLkQ+z2o ERC4eYlp+3WM9ZcTD/jOvgeSYzGodVE= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1689073403; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=J1Bb87U5x3S6hNg/bp/WXsFJEUGzju+SI4UnFd3g7VA=; b=+qW1IkCsPjC4zUbHjxtcJInZO3jPACZRYBMA2u3/fUGoQPj52yVv7p7canADfSBsWKcxme etW4wieFkYUAYADA== Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id E71902C143; Tue, 11 Jul 2023 11:03:22 +0000 (UTC) Date: Tue, 11 Jul 2023 11:03:22 +0000 (UTC) From: Richard Biener To: Tamar Christina cc: "gcc-patches@gcc.gnu.org" , nd , "jlaw@ventanamicro.com" Subject: RE: [PATCH 5/19]middle-end: Enable bit-field vectorization to work correctly when we're vectoring inside conds In-Reply-To: Message-ID: References: User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, 10 Jul 2023, Tamar Christina wrote: > > > - *type_out = STMT_VINFO_VECTYPE (stmt_info); > > > + if (cond_cst) > > > + { > > > + append_pattern_def_seq (vinfo, stmt_info, pattern_stmt, vectype); > > > + pattern_stmt > > > + = gimple_build_cond (gimple_cond_code (cond_stmt), > > > + gimple_get_lhs (pattern_stmt), > > > + fold_convert (ret_type, cond_cst), > > > + gimple_cond_true_label (cond_stmt), > > > + gimple_cond_false_label (cond_stmt)); > > > + *type_out = STMT_VINFO_VECTYPE (stmt_info); > > > > is there any vectype set for a gcond? > > No, because gconds can't be codegen'd yet, atm we must replace the original > gcond when generating code. > > However looking at the diff this code, don't think the else is needed here. > Testing an updated patch. > > > > > I must say the flow of the function is a bit convoluted now. Is it possible to > > factor out a helper so we can fully separate the gassign vs. gcond handling in > > this function? > > I am not sure, the only place the changes are are at the start (e.g. how we determine bf_stmt) > and how we determine ret_type, and when determining shift_first for the single use case. > > Now I can't move the ret_type anywhere as I need to decompose bf_stmt first. And the shift_first > can be simplified by moving it up into the part that determined bf_stmt, but then we walk the > immediate uses even on cases where we early exit. Which seems inefficient. > > Then there's the final clause which just generates an additional gcond if the original statement was > a gcond. But not sure that'll help, since it's just something done *in addition* to the normal assign. > > So there doesn't seem to be enough, or big enough divergence to justify a split. I have however made > an attempt at cleaning it up a bit, is this one better? Yeah, it is. > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > > Ok for master? OK. Richard. > Thanks, > Tamar > > gcc/ChangeLog: > > * tree-vect-patterns.cc (vect_init_pattern_stmt): Copy STMT_VINFO_TYPE > from original statement. > (vect_recog_bitfield_ref_pattern): Support bitfields in gcond. > > Co-Authored-By: Andre Vieira > > --- inline copy of patch --- > > diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc > index 60bc9be6819af9bd28a81430869417965ba9d82d..b842f7d983405cd04f6760be7d91c1f55b30aac4 100644 > --- a/gcc/tree-vect-patterns.cc > +++ b/gcc/tree-vect-patterns.cc > @@ -128,6 +128,7 @@ vect_init_pattern_stmt (vec_info *vinfo, gimple *pattern_stmt, > STMT_VINFO_RELATED_STMT (pattern_stmt_info) = orig_stmt_info; > STMT_VINFO_DEF_TYPE (pattern_stmt_info) > = STMT_VINFO_DEF_TYPE (orig_stmt_info); > + STMT_VINFO_TYPE (pattern_stmt_info) = STMT_VINFO_TYPE (orig_stmt_info); > if (!STMT_VINFO_VECTYPE (pattern_stmt_info)) > { > gcc_assert (!vectype > @@ -2441,6 +2442,10 @@ vect_recog_widen_sum_pattern (vec_info *vinfo, > bf_value = BIT_FIELD_REF (container, bitsize, bitpos); > result = (type_out) bf_value; > > + or > + > + if (BIT_FIELD_REF (container, bitsize, bitpos) `cmp` ) > + > where type_out is a non-bitfield type, that is to say, it's precision matches > 2^(TYPE_SIZE(type_out) - (TYPE_UNSIGNED (type_out) ? 1 : 2)). > > @@ -2450,6 +2455,10 @@ vect_recog_widen_sum_pattern (vec_info *vinfo, > here it starts with: > result = (type_out) bf_value; > > + or > + > + if (BIT_FIELD_REF (container, bitsize, bitpos) `cmp` ) > + > Output: > > * TYPE_OUT: The vector type of the output of this pattern. > @@ -2482,33 +2491,45 @@ vect_recog_widen_sum_pattern (vec_info *vinfo, > > The shifting is always optional depending on whether bitpos != 0. > > + When the original bitfield was inside a gcond then an new gcond is also > + generated with the newly `result` as the operand to the comparison. > + > */ > > static gimple * > vect_recog_bitfield_ref_pattern (vec_info *vinfo, stmt_vec_info stmt_info, > tree *type_out) > { > - gassign *first_stmt = dyn_cast (stmt_info->stmt); > - > - if (!first_stmt) > - return NULL; > - > - gassign *bf_stmt; > - if (CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (first_stmt)) > - && TREE_CODE (gimple_assign_rhs1 (first_stmt)) == SSA_NAME) > + gimple *bf_stmt = NULL; > + tree lhs = NULL_TREE; > + tree ret_type = NULL_TREE; > + gimple *stmt = STMT_VINFO_STMT (stmt_info); > + if (gcond *cond_stmt = dyn_cast (stmt)) > + { > + tree op = gimple_cond_lhs (cond_stmt); > + if (TREE_CODE (op) != SSA_NAME) > + return NULL; > + bf_stmt = dyn_cast (SSA_NAME_DEF_STMT (op)); > + if (TREE_CODE (gimple_cond_rhs (cond_stmt)) != INTEGER_CST) > + return NULL; > + } > + else if (is_gimple_assign (stmt) > + && CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (stmt)) > + && TREE_CODE (gimple_assign_rhs1 (stmt)) == SSA_NAME) > { > - gimple *second_stmt > - = SSA_NAME_DEF_STMT (gimple_assign_rhs1 (first_stmt)); > + gimple *second_stmt = SSA_NAME_DEF_STMT (gimple_assign_rhs1 (stmt)); > bf_stmt = dyn_cast (second_stmt); > - if (!bf_stmt > - || gimple_assign_rhs_code (bf_stmt) != BIT_FIELD_REF) > - return NULL; > + lhs = gimple_assign_lhs (stmt); > + ret_type = TREE_TYPE (lhs); > } > - else > + > + if (!bf_stmt > + || gimple_assign_rhs_code (bf_stmt) != BIT_FIELD_REF) > return NULL; > > tree bf_ref = gimple_assign_rhs1 (bf_stmt); > tree container = TREE_OPERAND (bf_ref, 0); > + ret_type = ret_type ? ret_type : TREE_TYPE (container); > > if (!bit_field_offset (bf_ref).is_constant () > || !bit_field_size (bf_ref).is_constant () > @@ -2522,8 +2543,6 @@ vect_recog_bitfield_ref_pattern (vec_info *vinfo, stmt_vec_info stmt_info, > > gimple *use_stmt, *pattern_stmt; > use_operand_p use_p; > - tree ret = gimple_assign_lhs (first_stmt); > - tree ret_type = TREE_TYPE (ret); > bool shift_first = true; > tree container_type = TREE_TYPE (container); > tree vectype = get_vectype_for_scalar_type (vinfo, container_type); > @@ -2560,7 +2579,7 @@ vect_recog_bitfield_ref_pattern (vec_info *vinfo, stmt_vec_info stmt_info, > /* If the only use of the result of this BIT_FIELD_REF + CONVERT is a > PLUS_EXPR then do the shift last as some targets can combine the shift and > add into a single instruction. */ > - if (single_imm_use (gimple_assign_lhs (first_stmt), &use_p, &use_stmt)) > + if (lhs && single_imm_use (lhs, &use_p, &use_stmt)) > { > if (gimple_code (use_stmt) == GIMPLE_ASSIGN > && gimple_assign_rhs_code (use_stmt) == PLUS_EXPR) > @@ -2620,6 +2639,19 @@ vect_recog_bitfield_ref_pattern (vec_info *vinfo, stmt_vec_info stmt_info, > NOP_EXPR, result); > } > > + if (!lhs) > + { > + append_pattern_def_seq (vinfo, stmt_info, pattern_stmt, vectype); > + gcond *cond_stmt = dyn_cast (stmt_info->stmt); > + tree cond_cst = gimple_cond_rhs (cond_stmt); > + pattern_stmt > + = gimple_build_cond (gimple_cond_code (cond_stmt), > + gimple_get_lhs (pattern_stmt), > + fold_convert (ret_type, cond_cst), > + gimple_cond_true_label (cond_stmt), > + gimple_cond_false_label (cond_stmt)); > + } > + > *type_out = STMT_VINFO_VECTYPE (stmt_info); > vect_pattern_detected ("bitfield_ref pattern", stmt_info->stmt); > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)