From patchwork Thu Jun 10 13:37:54 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 55221 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 6B4C0B7D8E for ; Thu, 10 Jun 2010 23:38:13 +1000 (EST) Received: (qmail 22891 invoked by alias); 10 Jun 2010 13:38:10 -0000 Received: (qmail 22869 invoked by uid 22791); 10 Jun 2010 13:38:08 -0000 X-SWARE-Spam-Status: No, hits=-6.1 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 10 Jun 2010 13:38:02 +0000 Received: from int-mx08.intmail.prod.int.phx2.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o5ADc1vo005943 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 10 Jun 2010 09:38:01 -0400 Received: from redhat.com (vpn-10-137.rdu.redhat.com [10.11.10.137]) by int-mx08.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o5ADbsVs018182 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Thu, 10 Jun 2010 09:37:57 -0400 Date: Thu, 10 Jun 2010 09:37:54 -0400 From: Aldy Hernandez To: Patrick Marlier Cc: Richard Henderson , FELBER Pascal , gcc-patches@gcc.gnu.org Subject: Re: [trans-mem] undefined reference to a static cloned function Message-ID: <20100610133753.GA13188@redhat.com> References: <4BF3C398.9090407@unine.ch> <4BF4F6FC.1040204@unine.ch> <20100607134205.GA17619@redhat.com> <4C0D0507.1070200@unine.ch> <20100607174417.GA13762@redhat.com> <4C0F727F.4010709@unine.ch> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4C0F727F.4010709@unine.ch> User-Agent: Mutt/1.5.20 (2009-08-17) Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org > gcc -fgnu-tm -Wall -DNDEBUG -O3 element.c -o element -litm -lm > /tmp/ccEhZPec.o:(.tm_clone_table+0x18): undefined reference to > `transaction clone for calculateCircumCircle' In the testcase below we have a clone that gets inlined and optimized away, so it doesn't get outputted, but we still dump its address into the clone table. I've fixed this by not dumping entries into the clone table for which clones are not needed. Previously we were looking at the neediness of the original function, but since we now mark clones as needed in ipa_tm_insert_gettmclone_call(), we are assured to have the needy bit set correctly. Also, I got tired of IPA dumps not distinguishing between regular functions and TM clones. I've made the CFG dumper distinguish between the two. OK for branch? * varasm.c (finish_tm_clone_pairs_1): * tree-cfg.c (dump_function_to_file): Display [tm-clone] if applicable. Index: testsuite/gcc.dg/tm/20100610.c =================================================================== --- testsuite/gcc.dg/tm/20100610.c (revision 0) +++ testsuite/gcc.dg/tm/20100610.c (revision 0) @@ -0,0 +1,90 @@ +/* { dg-do compile } */ +/* { dg-options "-fgnu-tm -O3" } */ + +/* The function calculateCircumCircle() should get inlined into the TM + clone for TMelement_alloc(), so we don't need to generate a TM + clone for calculateCircumCircle(). We also don't need to put its + entry into the clone table since it's static. */ + +/* { dg-final { scan-assembler-not "ZGTt21calculateCircumCircle" } } */ + +extern double sqrt(double) __attribute__((transaction_pure)); +extern void *xmalloc(int) __attribute__((transaction_safe)); + +typedef struct coordinate { + double x; + double y; +} coordinate_t; +typedef struct element { + coordinate_t coordinates[3]; + long numCoordinate; + coordinate_t circumCenter; + double circumRadius; +} element_t; + +__attribute__((transaction_safe)) +double +coordinate_distance (coordinate_t* coordinatePtr, coordinate_t* aPtr) +{ + return sqrt( coordinatePtr->x ); +} + +__attribute__((transaction_safe)) +static void +calculateCircumCircle (element_t* elementPtr) +{ + long numCoordinate = elementPtr->numCoordinate; + coordinate_t* coordinates = elementPtr->coordinates; + coordinate_t* circumCenterPtr = &elementPtr->circumCenter; + ((void) (0)); + if (numCoordinate == 2) { + circumCenterPtr->x = (coordinates[0].x + coordinates[1].x) / 2.0; + circumCenterPtr->y = (coordinates[0].y + coordinates[1].y) / 2.0; + } + else { + double ax = coordinates[0].x; + double ay = coordinates[0].y; + double bx = coordinates[1].x; + double by = coordinates[1].y; + double cx = coordinates[2].x; + double cy = coordinates[2].y; + double bxDelta = bx - ax; + double byDelta = by - ay; + double cxDelta = cx - ax; + double cyDelta = cy - ay; + double bDistance2 = (bxDelta * bxDelta) + (byDelta * byDelta); + double cDistance2 = (cxDelta * cxDelta) + (cyDelta * cyDelta); + double xNumerator = (byDelta * cDistance2) - (cyDelta * bDistance2); + double yNumerator = (bxDelta * cDistance2) - (cxDelta * bDistance2); + double denominator = 2 * ((bxDelta * cyDelta) - (cxDelta * byDelta)); + double rx = ax - (xNumerator / denominator); + double ry = ay + (yNumerator / denominator); + circumCenterPtr->x = rx; + circumCenterPtr->y = ry; + } + elementPtr->circumRadius = coordinate_distance(circumCenterPtr, + &coordinates[0]); +} + +element_t* +element_alloc (coordinate_t* coordinates, long numCoordinate) +{ + element_t* elementPtr; + elementPtr = (element_t*)xmalloc(sizeof(element_t)); + if (elementPtr) { + calculateCircumCircle(elementPtr); + } + return elementPtr; +} + +__attribute__((transaction_safe)) +element_t* +TMelement_alloc (coordinate_t* coordinates, long numCoordinate) +{ + element_t* elementPtr; + elementPtr = (element_t*)xmalloc(sizeof(element_t)); + if (elementPtr) { + calculateCircumCircle(elementPtr); + } + return elementPtr; +} Index: varasm.c =================================================================== --- varasm.c (revision 160538) +++ varasm.c (working copy) @@ -5850,9 +5850,15 @@ finish_tm_clone_pairs_1 (void **slot, vo bool *switched = (bool *) info; tree src = map->base.from; tree dst = map->to; - struct cgraph_node *src_n = cgraph_node (src); + struct cgraph_node *dst_n = cgraph_node (dst); - if (!src_n->needed) + /* The function ipa_tm_create_version() marks the clone as needed if + the original function was needed. But we also mark the clone as + needed if we ever called the clone indirectly through + TM_GETTMCLONE. If neither of these are true, we didn't generate + a clone, and we didn't call it indirectly... no sense keeping it + in the clone table. */ + if (!dst_n->needed) return 1; if (!*switched) Index: tree-cfg.c =================================================================== --- tree-cfg.c (revision 160538) +++ tree-cfg.c (working copy) @@ -6215,8 +6215,10 @@ dump_function_to_file (tree fn, FILE *fi bool ignore_topmost_bind = false, any_var = false; basic_block bb; tree chain; + bool tmclone = TREE_CODE (fn) == FUNCTION_DECL && DECL_IS_TM_CLONE (fn); - fprintf (file, "%s (", lang_hooks.decl_printable_name (fn, 2)); + fprintf (file, "%s %s(", lang_hooks.decl_printable_name (fn, 2), + tmclone ? "[tm-clone] " : ""); arg = DECL_ARGUMENTS (fn); while (arg)