From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1661 invoked by alias); 21 Jun 2011 16:47:48 -0000 Received: (qmail 1651 invoked by uid 22791); 21 Jun 2011 16:47:47 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,TW_IW,TW_MX X-Spam-Check-By: sourceware.org Received: from mail-qw0-f47.google.com (HELO mail-qw0-f47.google.com) (209.85.216.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 21 Jun 2011 16:47:32 +0000 Received: by qwh5 with SMTP id 5so1912333qwh.20 for ; Tue, 21 Jun 2011 09:47:31 -0700 (PDT) MIME-Version: 1.0 Received: by 10.224.175.211 with SMTP id bb19mr4951350qab.95.1308674851159; Tue, 21 Jun 2011 09:47:31 -0700 (PDT) Received: by 10.224.47.134 with HTTP; Tue, 21 Jun 2011 09:47:31 -0700 (PDT) In-Reply-To: References: <4737A960563B524DA805CA602BE04B306010BD2373@SC-VEXCH2.marvell.com> Date: Tue, 21 Jun 2011 16:56:00 -0000 Message-ID: Subject: Re: [PATCH, ARM] iWMMXT maintenance From: Ramana Radhakrishnan To: "Joseph S. Myers" Cc: Xinyu Qi , "gcc-patches@gcc.gnu.org" , ramana.radhakrishnan@arm.com Content-Type: text/plain; charset=ISO-8859-1 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-06/txt/msg01612.txt.bz2 Joseph S. Myers wrote: > On Mon, 20 Jun 2011, Xinyu Qi wrote: > >> *gcc/config/arm/elf.h: Add option -mwmmxt. >> *gcc/config/arm/arm.opt: Same. > > Why? And where are the documentation updates? How does this relate to > the existing iWMMXt options? > > I thought the plan (Ramana?) was to move to having a -msimd= option to > select SIMD units, reflecting that "neon" is not an FPU and "iwmmxt" is > not an architecture (or a CPU). > So you'd have -march=armv5te > -msimd=iwmmxt as replacement for the present -march=iwmmxt - or > -mcpu= implying that combination of options. Given that idea > I'm not sure adding another legacy option makes sense - at least the > option should have the desired -msimd=iwmmxt spelling. The -mwmmxt option is not acceptable as it stands today. IIRC the msimd option was the plan long term when we talked about this last. It is a good idea to revisit this now that we are finalizing the options / multilib rework and the iwmmx port is getting some maintenance. It would be preferable to no longer have to perpetuate the march=iwmxxt and mcpu=iwmmxt options if we can avoid it since they really aren't architectures or cpu's in the traditional sense. Joseph raises a valid point when he asks about the interaction with the current command line options. There is a lot of restructuring of pattern names in neon.md. When you say you tested arm-linux-gnueabi did you specifically test the neon port with your patches applied to be sure that nothing broke there since I notice this churn ? Based on a quick skim of the patch - In a number of places I noticed that you have For e.g. in your pipeline descriptions . ior (eq_attr ("wtype" "waligni") ior (eq_attr ("wtype" "walignr")) etc... You could rationalize these with eq_attr "wtype" "waligni, walignr" makes these things smaller :) Also based on a quick reading I find that 1. The documentation for the new intrinsics added is missing and that needs to be contributed along with the documentation to invoke.texi about the new options that are being added. 2. EABI build Attribute tags for wmmx2 aren't being added to the assembler output (maybe the assembler and readelf need to be taught about this). Look at the Addenda to the ELF ABI about attribute tags. The patch as it stands today is hard to review - can you break the patch into smaller chunks that will help review ? You could have a patch series roughly that does ( I haven't thought about it much since I haven't read your patch in great detail.) command line options changes target addressing changes one patch that reworks the neon backend and adds the new features of the vectorization bits of the iwmmxt .md a patch for the pipeline descriptions . documentation This allows the mechanical portions of your patch to be reviewed quicker than the other bits and potentially by more than one reviewer. cheers Ramana