[llvm-commits] ARM ELF disassembly with integrated-as

[Tim Northover]

Hi Greg,

On Friday 09 Nov 2012 22:14:53 Greg Fitzgerald wrote:
> Compared to the previous patch, this one is small, simple and comes with a
> unit-test.  It doesn’t move any files around and does its best to be a
> good citizen with regard to coding style.  Could you please review?

First, I'd say that I preferred the refactoring of the previous patch. I 
prefer to keep as much target-specific code as possible in its directory.

I'm afraid I disagree with ignoring ARM/Thumb switchover too. As Jan points 
out, that part of ARM assembly is supported, except that mapping symbols won't 
work with it.

As far as I can tell it would be fairly simple to implement. You just need to 
also override the EmitAssemblerFlag function in the ARMELFStreamer and do the 
right thing for MCAF_Code16 and MCAF_Code32.

You've also forgotten the tests for STT_NOTYPE and STB_LOCAL, and which tests 
are the ones that check the duplication code? Wouldn't you need two functions 
to force the Streamer to ChangeSection to .text twice?

Other than that, the patch seems reasonable to me. The symbols have the 
correct type and it's valid to omit them for data-only sections so I think the 
ABI is satisfied.

Regards.

Tim.