[PATCH] tools: support decoding ARM EHABI opcodes in readobj
Renato Golin
renato.golin at linaro.org
Sat Jan 18 05:02:53 PST 2014
Hi Saleem,
Overall, I like the patch. You seem to have some good tests, but you're not testing every decoding possible. If readobj doesn't mind bogus tables, I'd recommend to create a single table with one example of every pattern, to make sure we stress all code paths.
I like the use of a table dispatch, but some people are not comfortable with it. Since I have no say in readobj, I'll defer to other people to judge. There doesn't seem to be a code owner of the readobj, though...
From my perspective, with the added comments and tests, this patch looks good to go.
cheers,
--renato
================
Comment at: tools/llvm-readobj/ARMEHABIPrinter.h:273
@@ +272,3 @@
+ if (!Decoded)
+ SW.startLine() << format("0x%02X ; reserved\n", Opcodes[OCI++ ^ 3]);
+ }
----------------
You seem to be missing the spare encodings and treating as reserved. That's ok, but would be good to include it in the text above, like "reserved/spare".
http://llvm-reviews.chandlerc.com/D2565
More information about the llvm-commits
mailing list