[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