[llvm-commits] [patch] add support for a movi32 of a symbol to the ARM MC asm printer

[Chris Lattner]

On May 9, 2010, at 7:36 PM, Rafael Espindola wrote:

>> Right, this isn't the right way to go here.  In general, the MC stuff should use *fewer* instructions that codegen does.  Things like "the tail call form of return" etc go away with MC.
>> 
>> The right thing to do here depends on whether the reference to the global ends up with a special relocation type in the relocation entry.  Assuming it does, adding new VariantKinds is the right way to go (currently VariantKind is going to be a union of all target's relocation flags).  The best way to handle this is to have isel set these as TargetFlags when it lowers the instruction, then have ARM's MCInstLowering convert the target flags to the VariantKinds.
> 
> Ah, much better. The references do end up as different relocations
> (R_ARM_MOVW_ABS_NC and R_ARM_MOVT_ABS). There are not that many
> relocation types, so we might be able to have one enum for all
> targets.

Right.  Ok, but please add a comment in MCExpr saying what these are.  Also, since printing syntax is implied by the enum, it's probably best to name it VK_ARM_HI16/VK_ARM_LO16.  Please commit with this fix, thanks Rafael!



> The attached patch fixes the same issue as before, but now by updating
> VariantKind. There is currently no tests for
> -enable-arm-mcinst-printer. Should I add one or this is just something
> we just expect to make the default in a short(ish) time frame?

I would really like for it to finish (so that the old instruction printer can be expunged), but I got half done and then got distracted.  I probably won't get back to it for another month or two.  I was testing it by just running it over large programs in llvm-test and diff'ing the output with the old instprinter.  I don't think it was to the point yet where large programs were close to working, so no tests are really needed :)

-Chris