[cfe-commits] [PATCH] Fix NOP encodings in ARM backend.

[Kristof Beyls]

Hi,

With the attached patch, I'm trying to fix a FIXME in the ARM backend.  This
patch fixes ARMAsmBackend::WriteNopData, so that it takes into account the
version of the ARM architecture that is being targeted. For versions before
ARMv6T2, there is no NOP instruction, and NOPs are encoded as MOV r0,r0 (in
ARM
mode) or MOV r8,r8 (in Thumb mode). For targets later than ARMv6T2, the
encoding for the NOP instruction is created.

I have a few questions about this patch:

1. To make sure that ARMAsmBackend::WriteNopData can figure out which ARM
   sub-target it compiles for, I had to adapt the Target::MCAsmBackendCtorTy
to
   also pass on an MCSubtargetInfo argument. Is this the best way to get
   sub-target information to the ARMAsmBackend object?
   (this change results in a few function signature changes in the
    ARM, PowerPC, X86 and MBlaze backends).

2. It's hard to create test cases to test this properly, since I think
   that there is another bug in lib/MC/MCAssembler.cpp, where processing 
   an alignment fragment results in calling ARMAsmBackend::WriteNopData, but
   without putting the ARMAsmBackend in the right ARM or Thumb state.
   Therefore, e.g. when processing an assembler file with .align directives
   in the middle of a Thumb code section, still ARM NOP encodings are 
   generated instead of Thumb NOP encodings.
   Question 2a: Is it OK to write a FIXME to indicate this brokenness?
Should
   I also file a bugzilla issue?
   Question 2b: Is it OK to leave that fix for a later, separate, patch? For
   that fix, it will be easier to create good test cases that will also test
   this patch.

Thanks,

Kristof

PS. I'm cc-ing to the cfe-commits list because the change in
Target::MCAsmBackendCtorTy requires 2 lines to change in Clang too, see
attached file clang_arm_nop_encoding.patch.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm_arm_nop_encoding.patch
Type: application/octet-stream
Size: 15694 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110808/937ec169/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang_arm_nop_encoding.patch
Type: application/octet-stream
Size: 1155 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110808/937ec169/attachment-0001.obj>