[llvm-commits] [PATCH] ARM build attribute reading support

[Amara Emerson]

I've attached a new patch with some changes. Note that the patch is a git
binary diff because of the new test object files, but I can supply separate
patch + files if anyone wants me to.

They are:
 - Separate enum anonymous namespaces.
 - Public getters/setters have been removed completely to make the class
mostly just a data container.
 - A new large test case added which exercises the previously non-tested
attributes. llvm-readobj behaviour is congruent with readelf.

Michael, does this look reasonable?

Thanks,
Amara

-----Original Message-----
From: rengolin at gmail.com [mailto:rengolin at gmail.com] On Behalf Of Renato
Golin
Sent: 16 November 2012 08:53
To: Amara Emerson
Cc: llvm-commits at cs.uiuc.edu for LLVM
Subject: Re: [llvm-commits] [PATCH] ARM build attribute reading support

On 15 November 2012 18:18, Amara Emerson <amara.emerson at arm.com> wrote:
> Thanks for the comments. Yes, I realise the tests are a bit light at the
> moment. I was hoping to get some general functionality comments as it
would
> probably take a few hundred lines to test each attribute type (and bloat
the
> llvm-readobj tool somewhat). If the general structure looks good I can add
> more tests.

Should be enough to have a few objects with different configurations
for the attribute table on a test dir. And maybe one with a giant
table with all possible arguments in it.


>> Does big.LITTLE have its own value, here?
>
> No this is for the current ARM ELF ABI spec, which has a defined list of
> architecture types.

I know, but since big.LITTLE will certainly need new thoughts on how
to optimise and especially how to select libraries (probably something
like gnu's ifunc etc), would be good to have it in, if ARM already
have something public towards that.


> I don't have much of a preference really. The header file was an extended
> version of the existing one in the ARM backend so I continued with the
enum
> style already used.

I know, much of it was in the old file.


> I used getters/setters because I wasn't sure on how to do error handling
if
> an invalid or strange attribute value was read. libObject seems different
to
> the rest of LLVM in this sense. E.g. should reading an invalid/malformed
> attribute value result in an fatal error? If not, how should it warn or be
> handled silently. If anyone who knows it better than me can give tips here
> it'd be welcome.

My personal feeling is that a malformed attribute table should be
treated as an error, and you should not have setters in the class.

So, instead of transforming that into a POD, I'd leave it all private,
implement only the getters and let only that function create it
(either by marking its class as friend or by having a way to construct
it with all the information at once - maybe a factory).

The point is that we may want to merge different tables, but we don't
want to silently change any of the original tables, and each one must
reflect exactly what was in the object file. The factory mentioned
above could also hold the merging rules (when constructing from an
array of tables, for instance), if that proves necessary in the
future.

--
cheers,
--renato

http://systemcall.org/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: arm-buildattrs-r2.patch
Type: application/octet-stream
Size: 45016 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121119/cbb1f8be/attachment.obj>