From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 1D7E93858D28 for ; Thu, 16 Nov 2023 21:13:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1D7E93858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 1D7E93858D28 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700169206; cv=none; b=qcEFA9jf9OetqEP2dRu0EbVT9vqUC5lu/tFaONPtvtgx4VgoGGX+OHN0dMs+t3id8FxVig7Do1BpvV/0iVPCT1vxVyT8518jP4HmM6e39jwZl/HAFjUcp2hAfLP1idgJbJA4mksxn2RjAtwGFz7Bx3lgmqDGQmnGNNwsNJszYgQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700169206; c=relaxed/simple; bh=jBzDEiCXHzQOuce0te2VszLXJUddgrHvhq+APLj1r+o=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=NAAwzAdKK7AZXAuzdLLmkt0L5KU1d73ru9JZGSUf9Drhw1Tl6ZnoutMGg3mEfyFp6Gwjm4KIlntaddMPG0nrbExebDp2SW2wgV1ptRO7WRFSG/8U6FV/iEEdz2N5ru8YT0OwmuNjMRRtpVWKP8hkqMWrKsSfEZoSh7JDGwGrAEQ= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1700169204; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=wdC6boUks4cixCqRCDKOyntt8B05PF6XSTus6t00SjA=; b=UnqX8go2UyC2SalvuzMNadE2pyv2ZrLBjZ3Pgwr+HyL1OOJ5mTL+qX72RVHGRF5pa6++QF cBdqNlqQ630ow8zW4TIWSashZXkHFsk8lWgd0VoYmEXprQDYDeu3JRC/6fLjayNmk7OK7s C7eFJyRrNY9QIYBYzGXQjLvK6qoSQYU= Received: from mail-yw1-f200.google.com (mail-yw1-f200.google.com [209.85.128.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-152-_rF6sKa4M_uyiQwgiknb7w-1; Thu, 16 Nov 2023 16:13:23 -0500 X-MC-Unique: _rF6sKa4M_uyiQwgiknb7w-1 Received: by mail-yw1-f200.google.com with SMTP id 00721157ae682-5b31e000e97so19920277b3.1 for ; Thu, 16 Nov 2023 13:13:23 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700169202; x=1700774002; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=wdC6boUks4cixCqRCDKOyntt8B05PF6XSTus6t00SjA=; b=YasSizbG6Dmv2frQuaizUSkg+vPi9SWj1ElS8yv+uqdItyx8RCaqTZANIzUzBHGc6F jjzvEz8oZyB+SZqw2e9xFpV/Kxuq8u0foJW+PIqh0h6xmKKyn3lM3qlU/yRakqvMbTHo um5dEYN+7igYOKFg4mGjJ58up+phjErrQkFaW60bxtZXIWM4qNEA0d6iR4VDWexv/SuC ZYHP0+975UAerpNqrIJPzehlZpOCNIIZja+H5d+WGdaIAm0R4I/m+CxeZVqiCQkYLp1m H79uaU5F2iqJA0nIUExuyFYpFsU4BuBbES96cNkX5/og+hk4HfBdrws8CKDIIn3lDV9D rABQ== X-Gm-Message-State: AOJu0YxBvdDLnLPw5fctIDOkFhNL/Ny8dsSVI0J13Wnvgt6ViivhIjcy 3O2LKOkSRNcNAfDTmudM90XBscYxJdl8AY8Wt3VM2tE8FwVlO69TyBD0pytWSm3p3nvJ7hmFemT fYiNPYC+D/hptTs2+V8pAsKCcGQ== X-Received: by 2002:a81:8ac2:0:b0:595:59f:28d7 with SMTP id a185-20020a818ac2000000b00595059f28d7mr17996226ywg.48.1700169202401; Thu, 16 Nov 2023 13:13:22 -0800 (PST) X-Google-Smtp-Source: AGHT+IEnr/tdkCfUNcehuuOJTZtvsT8FqZLxmeIEyexO4fRTdii+cjo6N1fJ7L6hEBdxyacqAxflvg== X-Received: by 2002:a81:8ac2:0:b0:595:59f:28d7 with SMTP id a185-20020a818ac2000000b00595059f28d7mr17996200ywg.48.1700169202020; Thu, 16 Nov 2023 13:13:22 -0800 (PST) Received: from [192.168.1.88] (23-233-12-249.cpe.pppoe.ca. [23.233.12.249]) by smtp.gmail.com with ESMTPSA id f26-20020ad4559a000000b006710660a548sm82939qvx.27.2023.11.16.13.13.21 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 16 Nov 2023 13:13:21 -0800 (PST) Message-ID: <6827dadc-a39a-9267-b1f8-6f639ab2c1b8@redhat.com> Date: Thu, 16 Nov 2023 16:13:20 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH V3 4/7] ira: Support subreg copy To: Lehua Ding , gcc-patches@gcc.gnu.org Cc: richard.sandiford@arm.com, juzhe.zhong@rivai.ai References: <20231112120817.2635864-1-lehua.ding@rivai.ai> <20231112120817.2635864-5-lehua.ding@rivai.ai> From: Vladimir Makarov In-Reply-To: <20231112120817.2635864-5-lehua.ding@rivai.ai> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,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 11/12/23 07:08, Lehua Ding wrote: > This patch changes the previous way of creating a copy between allocnos to objects. > > gcc/ChangeLog: > > * ira-build.cc (find_allocno_copy): Removed. > (find_object): New. > (ira_create_copy): Adjust. > (add_allocno_copy_to_list): Adjust. > (swap_allocno_copy_ends_if_necessary): Adjust. > (ira_add_allocno_copy): Adjust. > (print_copy): Adjust. > (print_allocno_copies): Adjust. > (ira_flattening): Adjust. > * ira-color.cc (INCLUDE_VECTOR): Include vector. > (struct allocno_color_data): Adjust. > (struct allocno_hard_regs_subnode): Adjust. > (form_allocno_hard_regs_nodes_forest): Adjust. > (update_left_conflict_sizes_p): Adjust. > (struct update_cost_queue_elem): Adjust. > (queue_update_cost): Adjust. > (get_next_update_cost): Adjust. > (update_costs_from_allocno): Adjust. > (update_conflict_hard_regno_costs): Adjust. > (assign_hard_reg): Adjust. > (objects_conflict_by_live_ranges_p): New. > (allocno_thread_conflict_p): Adjust. > (object_thread_conflict_p): Ditto. > (merge_threads): Ditto. > (form_threads_from_copies): Ditto. > (form_threads_from_bucket): Ditto. > (form_threads_from_colorable_allocno): Ditto. > (init_allocno_threads): Ditto. > (add_allocno_to_bucket): Ditto. > (delete_allocno_from_bucket): Ditto. > (allocno_copy_cost_saving): Ditto. > (color_allocnos): Ditto. > (color_pass): Ditto. > (update_curr_costs): Ditto. > (coalesce_allocnos): Ditto. > (ira_reuse_stack_slot): Ditto. > (ira_initiate_assign): Ditto. > (ira_finish_assign): Ditto. > * ira-conflicts.cc (allocnos_conflict_for_copy_p): Ditto. > (REG_SUBREG_P): Ditto. > (subreg_move_p): New. > (regs_non_conflict_for_copy_p): New. > (subreg_reg_align_and_times_p): New. > (process_regs_for_copy): Ditto. > (add_insn_allocno_copies): Ditto. > (propagate_copies): Ditto. > * ira-emit.cc (add_range_and_copies_from_move_list): Ditto. > * ira-int.h (struct ira_allocno_copy): Ditto. > (ira_add_allocno_copy): Ditto. > (find_object): Exported. > (subreg_move_p): Exported. > * ira.cc (print_redundant_copies): Exported. > > --- > gcc/ira-build.cc | 154 +++++++----- > gcc/ira-color.cc | 541 +++++++++++++++++++++++++++++++------------ > gcc/ira-conflicts.cc | 173 +++++++++++--- > gcc/ira-emit.cc | 10 +- > gcc/ira-int.h | 10 +- > gcc/ira.cc | 5 +- > 6 files changed, 646 insertions(+), 247 deletions(-) The patch is mostly ok for me except that there are the same issues I mentioned in my 1st email. Not changing comments for functions with changed interface like function arg types and names (e.g. find_allocno_copy) is particularly bad.  It makes the comments confusing and wrong.  Also using just "adjust" in changelog entries is too brief.  You should at least mention that function signature is changed. > diff --git a/gcc/ira-build.cc b/gcc/ira-build.cc > index a32693e69e4..13f0f7336ed 100644 > --- a/gcc/ira-build.cc > +++ b/gcc/ira-build.cc > > diff --git a/gcc/ira-color.cc b/gcc/ira-color.cc > index 8aed25144b9..099312bcdb3 100644 > --- a/gcc/ira-color.cc > +++ b/gcc/ira-color.cc > @@ -20,6 +20,7 @@ along with GCC; see the file COPYING3. If not see > .... > > - ira_allocno_t next_thread_allocno; > + ira_object_t *next_thread_objects; > + /* The allocno all thread shared. */ > + ira_allocno_t first_thread_allocno; > + /* The offset start relative to the first_thread_allocno. */ > + int first_thread_offset; > + /* All allocnos belong to the thread. */ > + bitmap thread_allocnos; It is better to use bitmap_head instead of bitmap.  It permits to avoid allocation of bitmap_head for bitmap.  There are many places when bitmap_head in you patches can be better used than bitmap (it is especially profitable if there is significant probability of empty bitmap). Of  course the patch cab be committed when all the patches are approved and fixed.