[llvm-commits] [llvm] r98745 - in /llvm/trunk: lib/Target/ARM/ lib/Target/ARM/AsmPrinter/ test/CodeGen/ARM/ test/CodeGen/Thumb2/

[Bob Wilson]

On Mar 17, 2010, at 10:52 AM, Johnny Chen wrote:

> Author: johnny
> Date: Wed Mar 17 12:52:21 2010
> New Revision: 98745
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=98745&view=rev
> Log:
> Added sub-formats to the NeonI/NeonXI instructions to further refine the NEONFrm
> instructions to help disassembly.
> 
> We also changed the output of the addressing modes to omit the '+' from the
> assembler syntax #+/-<imm> or +/-<Rm>.  See, for example, A8.6.57/58/60.
> 
> And modified test cases to not expect '+' in +reg or #+num.  For example,
> 
> ; CHECK:       ldr.w	r9, [r7, #28]

Some more issues:

* The ARMInstPrinter changes should really have been a separate patch.  Besides not printing the extra pluses, you made a ton of other changes.  I assume these were syncing up with ARMAsmPrinter but I haven't checked that yet.  Can you explain?

* I don't like the changes to ARMInstrNEON.td.  There must be a better way to provide the information you need.  Please revert those changes until we work out something more suitable.

* There were some other changes mixed in that seem unrelated.  See below.....


> Modified: llvm/trunk/lib/Target/ARM/AsmPrinter/ARMAsmPrinter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/AsmPrinter/ARMAsmPrinter.cpp?rev=98745&r1=98744&r2=98745&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/AsmPrinter/ARMAsmPrinter.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/AsmPrinter/ARMAsmPrinter.cpp Wed Mar 17 12:52:21 2010
> @@ -612,10 +612,11 @@
> ARMAsmPrinter::printThumbITMask(const MachineInstr *MI, int Op) {
>   // (3 - the number of trailing zeros) is the number of then / else.
>   unsigned Mask = MI->getOperand(Op).getImm();
> +  unsigned CondBit0 = Mask >> 4 & 1;
>   unsigned NumTZ = CountTrailingZeros_32(Mask);
>   assert(NumTZ <= 3 && "Invalid IT mask!");
>   for (unsigned Pos = 3, e = NumTZ; Pos > e; --Pos) {
> -    bool T = (Mask & (1 << Pos)) == 0;
> +    bool T = ((Mask >> Pos) & 1) == CondBit0;
>     if (T)
>       O << 't';
>     else

What is this for??

> @@ -749,7 +750,18 @@
>   if (OffImm < 0)
>     O << "#-" << -OffImm;
>   else if (OffImm > 0)
> -    O << "#+" << OffImm;
> +    O << "#" << OffImm;
> +}
> +
> +void ARMAsmPrinter::printT2AddrModeImm8s4OffsetOperand(const MachineInstr *MI,
> +                                                     int OpNum) {
> +  const MachineOperand &MO1 = MI->getOperand(OpNum);
> +  int32_t OffImm = (int32_t)MO1.getImm() / 4;
> +  // Don't print +0.
> +  if (OffImm < 0)
> +    O << "#-" << -OffImm * 4;
> +  else if (OffImm > 0)
> +    O << "#" << OffImm * 4;
> }
> 
> void ARMAsmPrinter::printT2AddrModeSoRegOperand(const MachineInstr *MI,

printT2AddrModeImm8s4OffsetOperand used to do nothing.  Was that a bug?  Why did you add this?