From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1549 invoked by alias); 5 Jul 2007 18:17:28 -0000 Received: (qmail 1541 invoked by uid 22791); 5 Jul 2007 18:17:27 -0000 X-Spam-Check-By: sourceware.org Received: from smtp1.dnsmadeeasy.com (HELO smtp1.dnsmadeeasy.com) (205.234.170.134) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 05 Jul 2007 18:17:22 +0000 Received: from smtp1.dnsmadeeasy.com (localhost [127.0.0.1]) by smtp1.dnsmadeeasy.com (Postfix) with ESMTP id 594522FB036; Thu, 5 Jul 2007 18:17:20 +0000 (UTC) X-Authenticated-Name: js.dnsmadeeasy X-Transit-System: In case of SPAM please contact abuse@dnsmadeeasy.com Received: from avtrex.com (unknown [67.116.42.147]) by smtp1.dnsmadeeasy.com (Postfix) with ESMTP; Thu, 5 Jul 2007 18:17:20 +0000 (UTC) Received: from [192.168.7.26] ([192.168.7.26]) by avtrex.com with Microsoft SMTPSVC(6.0.3790.1830); Thu, 5 Jul 2007 11:17:21 -0700 Message-ID: <468D35AD.90208@avtrex.com> Date: Thu, 05 Jul 2007 18:23:00 -0000 From: David Daney User-Agent: Thunderbird 1.5.0.12 (X11/20070530) MIME-Version: 1.0 To: David Daney , Mark Mitchell , Paolo Bonzini , gcc-patches@gcc.gnu.org, rsandifo@nildram.co.uk Subject: Re: [Patch] 1/3 Add new builtin __builtin_flush_icache(). References: <468734D3.3020908@avtrex.com> <46875F54.8070905@gnu.org> <4687F6C1.6050906@avtrex.com> <46888752.2040207@gnu.org> <468ADFB4.4060400@codesourcery.com> <468BCF25.40807@avtrex.com> <468BD6CA.8000703@codesourcery.com> <468C999B.1000102@avtrex.com> <87vecy3f6y.fsf@firetop.home> In-Reply-To: <87vecy3f6y.fsf@firetop.home> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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: 2007-07/txt/msg00479.txt.bz2 Richard Sandiford wrote: > David Daney writes: >> +/* Expand a call to _builtin___clear_cache. If the target does not >> + support the operation, return NULL_RTX so it expands as a call. */ >> +static rtx >> +expand_builtin_clear_cache (tree exp) >> +{ >> + tree begin, end; >> + rtx begin_rtx, end_rtx; >> + >> + if (!targetm.builtin_clear_cache_inline_p()) >> + return NULL_RTX; >> + >> + /* We must not expand to a library call if >> + __builtin_clear_cache_inline_p() returns true. If we did, the >> + fallback library function in libgcc would expand to a call to >> + _builtin___clear_cache() and we would recurse infinitely. */ >> + if (!validate_arglist (exp, POINTER_TYPE, POINTER_TYPE, VOID_TYPE)) > > This comment seems to belong above the earlier code. However, I'm only > really replying because... > This is the way I meant for it to be. If targetm.builtin_clear_cache_inline_p() is false we just do the default expand to call the function in libgcc. If we are expanding in-line, the it is an error if the arguments are not pointers. The only reason it is an error is to prevent infinite recursion. If that were not an issue, we could return NULL_RTX if validate_arglist failed. >> + /* Evaluate BEGIN and END. */ >> + begin_rtx = expand_expr (begin, NULL_RTX, Pmode, EXPAND_NORMAL); >> + begin_rtx = convert_memory_address (Pmode, begin_rtx); >> + >> + end_rtx = expand_expr (end, NULL_RTX, Pmode, EXPAND_NORMAL); >> + end_rtx = convert_memory_address (Pmode, end_rtx); >> + >> +#ifdef HAVE_clear_cache >> + if (HAVE_clear_cache) >> + emit_insn (gen_clear_cache (begin_rtx, end_rtx)); >> +#endif > > When I said that the switch from general_operand to pmode_register_operand > would need target-independent changes, I meant it would need changes to > this bit of code. I think you need to check the predicates of each > clear_cache operand and force the operand into a register if it doesn't > match. Something like: > > if (HAVE_clear_cache) > { > icode = CODE_FOR_clear_cache; > if (!insn_data[icode].operand[0].predicate (begin_rtx, Pmode)) > begin_rtx = copy_to_mode_reg (Pmode, begin_rtx); > if (!insn_data[icode].operand[1].predicate (end_rtx, Pmode)) > end_rtx = copy_to_mode_reg (Pmode, end_rtx); > emit_insn (gen_clear_cache (begin_rtx, end_rtx)); > } > > (It's unfortunate that this construct has to be copied so often, > but that's the way it is.) > > As it stands, the code only seems to guarantee that the operand > is an address operand, whereas the predicates in the MIPS patch > claim that it will be a register. The operands in the MIPS patch are both (match_operand:SI 0 "general_operand"). Look at the part in mips.c where I do: begin = force_reg(Pmode, begin); If that is the wrong way to put something in a register I will change it. David Daney