This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH] gas/opcodes: Add initial arc nps400 support
- From: Andrew Burgess <andrew dot burgess at embecosm dot com>
- To: Claudiu Zissulescu <Claudiu dot Zissulescu at synopsys dot com>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>, "noamca at mellanox dot com" <noamca at mellanox dot com>, "Cupertino dot Miranda at synopsys dot com" <Cupertino dot Miranda at synopsys dot com>
- Date: Wed, 2 Mar 2016 19:37:11 +0000
- Subject: Re: [PATCH] gas/opcodes: Add initial arc nps400 support
- Authentication-results: sourceware.org; auth=none
- References: <1456443554-23475-1-git-send-email-andrew dot burgess at embecosm dot com> <098ECE41A0A6114BB2A07F1EC238DE8966179CDB at DE02WEMBXB dot internal dot synopsys dot com> <20160301151522 dot GQ9275 at embecosm dot com> <098ECE41A0A6114BB2A07F1EC238DE896617A02C at DE02WEMBXB dot internal dot synopsys dot com>
* Claudiu Zissulescu <Claudiu.Zissulescu@synopsys.com> [2016-03-01 20:53:25 +0000]:
> Hi Andrew,
> > So do you see the 'ARC_CPU_DEFAULT' being defined to 'ARC700' for the
> > current stock arc binutils, and then set to 'NPS400' for the Mellanox
> > NPS400 binutils?
> No, more like arc700 or archs. The nps400 is an arc700 with some
> extra instructions. What I am trying to do is to have a piece of
> code that is generic and mps400 is only one of the possible
> configurations. But this discussion may be for another time.
Given that you said in a previous mail to make the default for core
arc binutils be arc700, which suits nps400 fine, I think that this
change can be put off for another day. I already have a test in place
that ensures that nps400 gets arc700 as the default, so if/when the
rest of the arc binutils wants a different default, we can make the
above change then.
> > That leaves just a single use of ARC_NPS400 in the opcode library,
> > where we include the extension instructions. One possibility would be
> > that I could create a define call ARC_OPCODE_EXTENSION_INSN_FILE or
> > some such. This would be undefined for core arc, and defined to
> > "arc-nps400-tbl.h" for nps400, then in arc_opcodes we can say:
> > #ifdef ARC_OPCODE_EXTENSION_INSN_FILE
> > #include ARC_OPCODE_EXTENSION_INSN_FILE
> > #endif
> > Would that be better?
> Looks nice. You need to think how to test the nsp400 without
> special actions, if possible.
My intention is that testing nps400 would be done by configuring and
building the nps400 target. All the arc/nps400 tests will be guarded
so they don't run on core arc binutils, so we'll see no additional
test failures (if you do this is a mistake on my part, please let me
> > > Probably the best will be to make a file common for all other users
> > > of ARC architecture which are having their own custom
> > > instructions. Hence, I would propose to rename arc-nps400-tbl.h to
> > > arc-ext-tbl.h or something of a sort.
> > I'm not sure I like that idea. Keeping different extensions separate
> > would be best I think.
> > The total number of extensions is likely to be pretty low, but the
> > number of entries in (this one at least) is likely to be ~100.
> > Keeping them separate will help provide more structure to the code,
> > making it more obvious which sets of extensions go together.
> I see your point, I just want to avoid to have a separate file for
> each ARC custom configuration in the binutils (see my generality
> concern). Again, it is not a show stopper.
I think that if there's a significant amount of code, which there will
be in this case, and it's logically separate from other, similar code,
then placing it in a separate file is probably best. I don't think we
should worry about too many files; if there really are that many arc
variants out there trying to get into binutils we can always move the
tables to support them into a sub-directory. As you say, this should
probably wait until it becomes an issue.
> > > In this
> > > situation, the disassembler may choose the first ones instead of
> > > yours. However, in this case, we need to add an option to the
> > > disassembler as well.
> > If this ever became a problem then I think the correct solution would
> > be a new ELF header flag, in the same way we split ARCv2 in to ARCEM
> > and ARCHS, we'd split ARC700 into ARC700 and NPS400. I'm hoping any
> > such flag changes can be put off until necessary.
> Maybe the best is to have a section with all the relevant
> information in. However, as you show it, we are not in need of such
> extra addition to the toolchain.
OK, for now I'll not worry about this. We can address it later if
Thanks for the review. I'll roll a new version of the patch, and post