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

[Johnny Chen]

The ARMInstPrinter changes are to sync up with the ARMAsmPrinter.cpp changes.

I'm currently working on the ARMInstrNEON.td modification to take advantage of the extra bit in Format.
Can I keep it as it is for now?

The printThumbITMask() thing also sneaked in without the accompanying
Thumb2ITBlockPass patch.  What would you like me to do?  Revert the printThumbITMask() patch
for ARMAsmPrinter.cpp as well as ARMInstPrinter, or just the ARMAsmPrinter one?

 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Thumb2ITBlock-patch.diff
Type: application/octet-stream
Size: 716 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100317/5a428b5c/attachment.obj>
-------------- next part --------------


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

This is to sync up with the ARMInstPrinter one.  It's used by the disassembly, but it could potentially
be used by the ARMAsmPrinter.

On Mar 17, 2010, at 3:22 PM, Bob Wilson wrote:

> 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?
>