From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5396 invoked by alias); 4 Apr 2016 09:10:07 -0000 Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org Received: (qmail 5382 invoked by uid 89); 4 Apr 2016 09:10:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=Hx-languages-length:4526, Allowing, HTo:D*synopsys.com, clarify X-HELO: mail-lb0-f174.google.com Received: from mail-lb0-f174.google.com (HELO mail-lb0-f174.google.com) (209.85.217.174) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 04 Apr 2016 09:09:56 +0000 Received: by mail-lb0-f174.google.com with SMTP id vo2so150994432lbb.1 for ; Mon, 04 Apr 2016 02:09:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=ffFzS70ETWh8uN2IJAvrwvdAeWHiFz9uPHfqgsUOgPc=; b=AZ3IPFr1cv/H3C86h8/skB8tPk/3gmu9y7xYLQmigbMkDaIM2vGmwQpIRGfDZqAoFC r3BLINK54364ZfPDkHnVaD7dEyDza4t6DMcb8rvgl+H0C8hxVgQD8C6BpExvV70pJ1Zr jOneMoETtfKJdWOFUPf6A3d4W8skKg+8gKE4ZquSC6hZ2AKVWFSU1XdjMzDOkhbzNcJD hZTex1fD60ITpSNJC/aDbLCVAhy1D4RJ+LNWLEfJOUqJla1IAH5jBLntW+RTdBITJ4yS n+guLda/N7RqrR8966yBY7k2sAQTbybcbYA6HJsWgo7L94Xua9AiBgwZksYqPJKShSQQ lcgg== X-Gm-Message-State: AD7BkJLpumduHJ1vnVIUaPk+Q08IYTLSjQk1vTR54g9Y6kS6KFinvb0EVNKe9ASMM+k3uQ== X-Received: by 10.28.91.83 with SMTP id p80mr11102323wmb.48.1459760991889; Mon, 04 Apr 2016 02:09:51 -0700 (PDT) Received: from localhost (host81-140-212-51.range81-140.btcentralplus.com. [81.140.212.51]) by smtp.gmail.com with ESMTPSA id b1sm28046013wjy.0.2016.04.04.02.09.50 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 04 Apr 2016 02:09:51 -0700 (PDT) Date: Mon, 04 Apr 2016 09:10:00 -0000 From: Andrew Burgess To: Claudiu Zissulescu Cc: "binutils@sourceware.org" , "noamca@mellanox.com" , Francois Bedard Subject: Re: [PATCH 1/6] gas/arc: Modify structure used to hold opcodes Message-ID: <20160404090935.GD25615@embecosm.com> References: <1459637470-30538-1-git-send-email-andrew.burgess@embecosm.com> <098ECE41A0A6114BB2A07F1EC238DE896617CF7D@DE02WEMBXB.internal.synopsys.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <098ECE41A0A6114BB2A07F1EC238DE896617CF7D@DE02WEMBXB.internal.synopsys.com> X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.5.24 (2015-08-30) X-IsSubscribed: yes X-SW-Source: 2016-04/txt/msg00046.txt.bz2 * Claudiu Zissulescu [2016-04-04 08:21:11 +0000]: > Hi Andrew, > > > The assembler builds a hash to hold references to arc_opcode entries in > > the arc_opcodes table. This hash assumes that each mnemonic will always > > appear in contiguous blocks within the arc_opcodes table, so all ADD > > instruction will be together, all AND instructions will likewise be > > together and so on. > > Yes this is on purpose. Having the group, we ensure we choose the > correct encoding. As you can see the grouping of a mnemonic is done > accordingly to an order, such that the small immediate arguments are > preferred before the long ones. Once, breaking that you need to do > the sorting at runtime, which will increase the time a file is > assembled. I see what you mean, though I slightly disagree, it's the ordering, not the grouping that is relied upon. So long as any second group only supports instructions that are not covered by an earlier group then all will be fine. Given that the arc assembler as currently written relies on the programmer to get the ordering "just right" I don't (personally) see restricting to one group as being that much of an advantage in getting the ordering correct. I guess you disagree, but > > > > > The problem with this is that as different variations of arc are added, > > then it is often more convenient to split instructions apart, so all the > > base ADD instructions are together, but, variants of ADD specific to one > > variation of arc are grouped with other instructions specific to that > > arc variant. The current data structures don't support this > > configuration of instructions. > > It may not be a good idea to overwrite the basic ARC ISA. Any ARC > customer can choose to have a different name than the standard ISA > names. Please can u reconsider this. I don't really understand what you are suggesting here, but ... > I do not see any reason why a > customer to use ADD mnemonic and not MYADD or something of a > sort. ... I think you're saying, change the ISA to work around limitation in the assembler :-/ That doesn't feel like a good answer, surely we should just adapt the assembler until it is flexible enough to handle whatever ISAs we need. > Allowing the overwriting/addnotation of the standard ISA > mnemonics is opening a domain of pain. On inspection you'll notice that the new instruction actually uses a unique flag operand and so there will never be a chance of confusion in this case. Could you clarify who you think will suffer this "domain of pain"? The tool-chain maintainer? The customer of Synopsys who is adding the new instructions? The user of the final tools? Or someone else? In the example you gave above, if a customer can think of a new ADD instruction that fills a gap not covered by the core ADD instruction then I think they absolutely _should_ call it ADD, not MYADD. Consider the end user of the tools, if this new ADD is just like any other ADD, but happens to take different parameter types, then having this one variant being called MYADD would be highly annoying, and would feel like the instruction was glued onto the side of the architecture, rather than being seamlessly integrated into it. I think that we can, and should, do better. > On top of everything, I miss > tests for your proposed patch (1 to 4). Sorry I was not explicit in the commit message, but there should be no functional change from patches 1 through 4. Patch 5 should have no functional change given the current state of the assembler. These tests are therefore that everything that currently works should continue to do so. It is only in patch 6 where I add a new instruction (that relies on patches 1 to 5) that I add a new test. I think I see 3 possible ways forward: 1. Keep the current patch series, understanding that the ordering restriction still applies even across disjoint groups of instructions, and accepting that as a know danger area in the assembler. This would be my preferred solution. 2. Move this one nps instruction (sync.{rd,wr}) into the main arc opcode table, placing it next the core 'sync' instruction. I'd probably leave a comment in the nps decode table explaining this strange placement. 3. Write a small program that reads in the arc decode tables and places the entries into a single unified order, which can then be read by the assembler and disassembler. Interested to hear your thoughts, thanks, Andrew