From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30009 invoked by alias); 4 Jan 2013 14:45:43 -0000 Received: (qmail 29999 invoked by uid 22791); 4 Jan 2013 14:45:43 -0000 X-SWARE-Spam-Status: No, hits=-3.1 required=5.0 tests=AWL,BAYES_00,KHOP_SPAMHAUS_DROP,KHOP_THREADED,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mms3.broadcom.com (HELO mms3.broadcom.com) (216.31.210.19) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 04 Jan 2013 14:45:38 +0000 Received: from [10.9.200.133] by mms3.broadcom.com with ESMTP (Broadcom SMTP Relay (Email Firewall v6.5)); Fri, 04 Jan 2013 06:40:26 -0800 X-Server-Uuid: B86B6450-0931-4310-942E-F00ED04CA7AF Received: from mail-irva-13.broadcom.com (10.11.16.103) by IRVEXCHHUB02.corp.ad.broadcom.com (10.9.200.133) with Microsoft SMTP Server id 8.2.247.2; Fri, 4 Jan 2013 06:45:13 -0800 Received: from [10.177.73.43] (unknown [10.177.73.43]) by mail-irva-13.broadcom.com (Postfix) with ESMTP id 7138C40FE3; Fri, 4 Jan 2013 06:45:24 -0800 (PST) Message-ID: <50E6EB05.8010600@broadcom.com> Date: Fri, 04 Jan 2013 14:45:00 -0000 From: "Andrew Burgess" User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: "Ulrich Weigand" cc: tromey@redhat.com, gdb-patches@sourceware.org, ken@linux.vnet.ibm.com Subject: Re: [PATCH] Vector to scalar casting and widening References: <201211291749.qATHnbBM030788@d06av02.portsmouth.uk.ibm.com> In-Reply-To: <201211291749.qATHnbBM030788@d06av02.portsmouth.uk.ibm.com> Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2013-01/txt/msg00066.txt.bz2 On 29/11/2012 5:49 PM, Ulrich Weigand wrote: > Andrew Burgess wrote: >> On 26/11/2012 2:18 PM, Ulrich Weigand wrote: >>> Thus, I'd prefer a patch that: >>> - changes the default behaviour of value_cast etc. to the GCC behaviour, and >>> - adds handling of UNOP_CAST etc. to evaluate_subexp_opencl to implement the >>> OpenCL semantics. >> >> The latest patch below is is closer to the initial patch I posted, the >> behaviour of value_cast now follows GCC, and the UNOP_CAST case is handled >> in evaluate_subexp_opencl. I wasn't sure what the "etc" in the above point >> referred to, I believe UNOP_CAST is all I need to catch, let me know if I've >> missed anything. > > Well, there's the whole "vector scalar" issue; with your patch, this > is now only supported if the scalar type is widened, and rejected if it is > truncated. This does not correspond to OpenCL semantics, where truncation > is explicitly supported. > > Another problem: in several places in opencl-lang.c, the code calls > value_cast with the explicit assumption that this will splat a scalar > across all vector components. Those uses would now have to be changed > to a value_cast/value_vector_widen pair, I guess. I finally found time to revisit this issue. I'm having issues with the approach you suggested, that is adding handling for UNOP_CAST into evaludate_subexp_opencl, and would welcome further advice on how I might move this issue forward. In addressing the two problems you pointed out above, I first created a new function opencl_value_cast, which I use throughout opencl-lang.c, however, a quick grep of the codebase shows other places that call value_cast, for example, within the varobj.c code. I suspect that creating an opencl_value_cast function will mean that I am introducing bugs when, for example, we create varobj objects for opencl code. The first issue you pointed out above "vector scalar" seems even harder to fix. The current problem with my patch is that I call value_vector_widen from value_binop, which will error if the scalar value is truncated when casting to the vector type. For OpenCL this is the wrong thing to do. I don't want to add an "if (language == opencl)" switch to value_binop, and value_binop is called extensively throughout the codebase. This would suggest then that it is value_vector_widen that should change, however, this feels very much like the original patch to which you objected. So, to summarise the problem as I see it, your suggested solution was to filter the different behaviours within opencl-lang.c, however, the problem behaviours live within value_cast and value_binop, both of which I believe are called from core gdb code, not just from opencl-lang.c, I therefore believe there's no reliable way to intercept all calls to these functions. I'd like to put two other possible solutions on the table (they are really just variations on the same theme), these would be (1) go back to my original flag on the language structure, handle all the different variations within value_cast or value_vector_widen, or (2) have a set of "vector_ops" function pointers on the language structure, currently there would be just two, "scalar_to_vector_widen" and "scalar_to_vector_cast". The defaults would be standard gcc C behaviour implemented in (probably) valops.c, while opencl would provide its own within opencl-lang.c. Alternatively I may have just miss-understood the issue here, in which case feel free to set me straight! Thanks for your time, Andrew > >>> Unfortunately the only environment I know of to test the GDB OpenCL support >>> is the IBM OpenCL SDK for PowerPC. I'll be happy to run the test for you ... >> >> I've built and tested on x86-64 GNU/Linux, but I've not run any OpenCL tests. If >> you were able to give this patch a go and let me know if I've broken any of the >> OpenCL tests I'd be very grateful. > > As expected, given the above, a whole bunch of tests fail ... > > Bye, > Ulrich >