[llvm-commits] Fixing Bug 13662: paired register for inline asm with 64-bit data on ARM

[Jim Grosbach]

Hi Weiming,

You may want to consider a RegisterOperand here, which allows a custom print method on a register class. You can just use the RegisterOperand wherever you'd normally use a RegisterClass in the (ins) and (outs) lists. For example,
def GPRPairOp : RegisterOperand<GPRPair, "printGPRPair">;

Then the printOperand() method doesn't need to perform the special "is this a gprpair?" check for every register operand, which is a speed improvement on the common path. There's a dedicated print method that's only used for these operands.

The asm parser bit seems ok. It's a pretty ugly hack, but that's somewhat necessary at this stage, as the parsers don't have any way to express these kinds of constraints, so we have to do it manually in C++ code. Please add a FIXME there with a very descriptive comment about why this is necessary.

-Jim

On Nov 1, 2012, at 3:35 PM, Weiming Zhao <weimingz at codeaurora.org> wrote:

> Hi Jakob & Jim,
> 
> I updated the instruction def of ldrexd/strexd to let them use GPRPair Reg
> class.
> I also fixed ldrexd/strexd intrinsics and atomic_64 lowering accordingly.
> Disassembler and AsmParser are adjusted to fit the new instruction def.
> The original test case "atomic-64bit.ll" is updated, so it's not checking
> hardcoded R0,R1 or R2,R3
> 
> It passes the "make check", especially atomic-64bit.ll, ldstrexd,ll,
> MC/ARM/basic-asm-instrucions.s, MC/ARM/diagnstics.s and MC/Disassembler/ARM/
> basic-arm-instructions.txt
> 
> Please help to review it.
> 
> Thanks,
> Weiming
> 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
> The Linux Foundation
> 
> 
> -----Original Message-----
> From: Jim Grosbach [mailto:grosbach at apple.com] 
> Sent: Wednesday, October 31, 2012 3:24 PM
> To: Jakob Stoklund Olesen
> Cc: weimingz at codeaurora.org; llvm-commits at cs.uiuc.edu; zinob at codeaurora.org
> Subject: Re: [llvm-commits] Fixing Bug 13662: paired register for inline asm
> with 64-bit data on ARM
> 
> 
> On Oct 31, 2012, at 3:17 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
> 
>> 
>> On Oct 31, 2012, at 3:15 PM, "Weiming Zhao" <weimingz at codeaurora.org>
> wrote:
>> 
>>> Hi Jakob,
>>> 
>>> I'm changing the ldrexd/strexd of ARMInstrInfo.td to def LDREXD: 
>>> AIldrex<0b01, (outs GPRPair:$Rt),(ins addr_offset_none:$addr),
>>>                    NoItinerary, "ldrexd", "\t$Rt, $addr", []> { let 
>>> DecoderMethod = "DecodeDoubleRegLoad"; }
>>> 
>>> def STREXD : AIstrex<0b01, (outs GPR:$Rd),
>>>                  (ins GPRPair:$Rt, addr_offset_none:$addr),
>>>                  NoItinerary, "strexd", "\t$Rd, $Rt, $addr", []> { 
>>> let DecoderMethod = "DecodeDoubleRegStore"; }
>>> 
>>> Is this the way you're referring to?
>> 
>> Exactly.
>> 
>>> So far, it almost works but I need to deal with printing "R0_R1" for 
>>> the GPRPair reg name. I'm planning to do it in 
>>> InstPrinter/ARMInstPrinter.cpp::printOperand().
>> 
>> I think that's right. Jim?
> 
> Sure, that'll work. Changing the instruction definitions like this will
> require some work in the AsmParser, too, though. I'm surprised you're not
> seeing a "make check" failure on basic-arm-instructions.s with these
> changes.
> 
> -Jim
> <0002-Remove-hard-coded-registers-in-ARM-ldrexd-and-strexd.patch>