From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3136 invoked by alias); 7 Nov 2011 09:45:11 -0000 Received: (qmail 3119 invoked by uid 22791); 7 Nov 2011 09:45:10 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,TW_TM X-Spam-Check-By: sourceware.org Received: from mail-qy0-f175.google.com (HELO mail-qy0-f175.google.com) (209.85.216.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 07 Nov 2011 09:44:56 +0000 Received: by qyc1 with SMTP id 1so2159487qyc.20 for ; Mon, 07 Nov 2011 01:44:56 -0800 (PST) MIME-Version: 1.0 Received: by 10.182.232.38 with SMTP id tl6mr7298411obc.22.1320659096028; Mon, 07 Nov 2011 01:44:56 -0800 (PST) Received: by 10.182.17.232 with HTTP; Mon, 7 Nov 2011 01:44:55 -0800 (PST) In-Reply-To: <4EB6D797.4070309@redhat.com> References: <4EB2EC3F.6000908@redhat.com> <4EB6D797.4070309@redhat.com> Date: Mon, 07 Nov 2011 09:48:00 -0000 Message-ID: Subject: Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH) From: Richard Guenther To: Aldy Hernandez Cc: gcc-patches , Richard Henderson Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-11/txt/msg00885.txt.bz2 On Sun, Nov 6, 2011 at 7:53 PM, Aldy Hernandez wrote: > [rth, more comments for you below] > > On 11/04/11 04:14, Richard Guenther wrote: > >>> =A0 =A0new_version =3D cgraph_create_node (new_decl); >>> >>> - =A0 new_version->analyzed =3D true; >>> + =A0 new_version->analyzed =3D old_version->analyzed; >> >> Hm? =A0analyzed means "with body", sure you have a body if you clone. >> >>> =A0 =A0new_version->local =3D old_version->local; >>> =A0 =A0new_version->local.externally_visible =3D false; >>> =A0 =A0new_version->local.local =3D true; >>> @@ -2294,6 +2294,7 @@ cgraph_copy_node_for_versioning (struct >>> =A0 =A0new_version->rtl =3D old_version->rtl; >>> =A0 =A0new_version->reachable =3D true; >>> =A0 =A0new_version->count =3D old_version->count; >>> + =A0 new_version->lowered =3D true; >> >> OTOH this isn't necessary true. =A0cgraph exists before lowering. > > I don't understand what you want me to do on either of these two comments. > =A0Could you elaborate? Do not push new_version->lowered setting into the core routine but keep it at the callers. >>> + =A0/* TM pure functions should not get inlined if the outer function = is >>> + =A0 =A0 a TM safe function. =A0*/ >>> + =A0else if (flag_tm >> >> Please move flag checks into the respective prediates. =A0Any reason >> why the is_tm_pure () predicate wouldn't already do the correct thing >> with !flag_tm? > > Done. > >>> + =A0/* Map gimple stmt to tree label (or list of labels) for transacti= on >>> + =A0 =A0 restart and abort. =A0*/ >>> + =A0htab_t GTY ((param_is (struct tm_restart_node))) tm_restart; >>> + >> >> As this maps 'gimple' to tree shouldn't this go to fn->gimple_df instead? >> That way you avoid growing generic struct function. =A0Or in to eh_statu= s, >> if that looks like a better fit. > > Done. > >>> + =A0/* Mark all calls that can have a transaction restart. =A0*/ >> >> Why isn't this done when we expand the call? =A0This walking of the >> RTL sequence looks like a hack (an easy one, albeit). >> >>> + =A0if (cfun->tm_restart&& =A0is_gimple_call (stmt)) >>> + =A0 =A0{ >>> + =A0 =A0 =A0struct tm_restart_node dummy; >>> + =A0 =A0 =A0void **slot; >>> + >>> + =A0 =A0 =A0dummy.stmt =3D stmt; >>> + =A0 =A0 =A0slot =3D htab_find_slot (cfun->tm_restart,&dummy, NO_INSER= T); >>> + =A0 =A0 =A0if (slot) >>> + =A0 =A0 =A0 { >>> + =A0 =A0 =A0 =A0 struct tm_restart_node *n =3D (struct tm_restart_node= *) *slot; >>> + =A0 =A0 =A0 =A0 tree list =3D n->label_or_list; >>> + =A0 =A0 =A0 =A0 rtx insn; >>> + >>> + =A0 =A0 =A0 =A0 for (insn =3D next_real_insn (last); !CALL_P (insn); >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0insn =3D next_real_insn (insn)) >>> + =A0 =A0 =A0 =A0 =A0 continue; >>> + >>> + =A0 =A0 =A0 =A0 if (TREE_CODE (list) =3D=3D LABEL_DECL) >>> + =A0 =A0 =A0 =A0 =A0 add_reg_note (insn, REG_TM, label_rtx (list)); >>> + =A0 =A0 =A0 =A0 else >>> + =A0 =A0 =A0 =A0 =A0 for (; list ; list =3D TREE_CHAIN (list)) >>> + =A0 =A0 =A0 =A0 =A0 =A0 add_reg_note (insn, REG_TM, label_rtx (TREE_V= ALUE (list))); >>> + =A0 =A0 =A0 } >>> + =A0 =A0} > > I can certainly move this to expand_call_stmt() if you prefer. =A0Do you = have > an objection to the RTL walk? =A0This isn't my code, but I'm open to > suggestions on an alternative to implement. It just catched my eye... moving it to expand_call_stmt would be nice indeed, but I was suggesting to add that note where we produce the CALL rtx, not sure if that's reasonably straight-forward (I suppose there was a reason to go with the hack above ...). Richard.