This is the mail archive of the mailing list for the binutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: introduce SH 2a support

I have not tested or checked this patch in detail, but there are two
issues I notice straight off:

1) The inheritance tree is now badly broken.

You have not updated the inheritance diagram in sh-opc.h so it is
difficult to see exactly what you have done, but some analysis reveals
the problems. This diagram was put in to try to make sense of the code,
and in the process identified some problems - thus proving its value.
Besides which, if it is out of date it only creates confusion.

The instruction table now contains entries such as

	arch_sh4_up | arch_sh2a_up

which introduces ambiguity when an instruction was introduced into two
architectures independently: the assembler and linker will not know
which architecture they are dealing with. This will probably work until
they have to choose what arch to put on the output file and then you
will get pot luck (and it won't work the same once loaded back in).

The only solution is to invent one or more imaginary architectures (c.f
sh3-nommu) in order to maintain a proper inheritance tree in which each
and every instruction may be traced to a common ancestor.

It looks like this new monster will cause some pretty nasty changes.

Once this is properly resolved there will no longer be any need to
always pass -isa=sh2a from the compiler (except where it is desirable to
limit the code to that architecture and no other, or in the case of
architecture specific relaxation optimisations etc.)

2) You have not added test cases to gas/testsuite/gas/sh/arch or
ld/testsuite/ld-sh/arch . These directories perform permutation tests of
the -isa assembler option and input variations to both the assembler and
linker. These algorithms are _very_ sensitive to changes in the
inheritance tree. You should add a file to each which uniquely
identifies each of your new architectures. You should then run the
testsuite, painstakingly check the output file, and update the expected
results to match if everything was correct. Obviously there should be
little change to the existing permutations (none if you have not
inserted anything above existing architectures), but the new stuff will
get inserted in among the old.

I assume that, since you have added new test cases elsewhere, you have
run the testsuite, including my arch tests, and there were no
regressions. This surprises me slightly, but those tests don't use many
exotic instructions.

No doubt Joern will follow up with a more detailed analysis of the rest
of the patches.

Andrew Stubbs
SuperH (UK) Ltd.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]