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

[Weiming Zhao]

HiJakob,

Thanks for your quick feedback.
It's a good idea to split it into several pieces. 

Now, I'm planning to isolate the first patch for GPR Pair alone (including
td file, reserved regs, support for copying and spilling). 
However, one obstacle is unit testing. Without the inline asm fix as the
"driver", how do I test the functionality of GPR pair? 
Any suggestions? Or shall we delay the unit test until the picture is more
completed?

Thanks a lot,
Weiming

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation


-----Original Message-----
From: Jakob Stoklund Olesen [mailto:stoklund at 2pi.dk] 
Sent: Tuesday, October 16, 2012 3:54 PM
To: weimingz at codeaurora.org
Cc: 'Jim Grosbach'; 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 15, 2012, at 10:58 AM, Weiming Zhao <weimingz at codeaurora.org> wrote:

> Hi Jakob,
> 
> Thanks for your guide.
> I'm attaching a new patch that is completely re-implemented by using 
> GPRPair as you suggested.
> Please help to review it.

Hi Weiming,

Let's break this into multiple patches.

First, add the GPRPair register class as well as support for copying and
spilling GPRPair registers in copyPhysReg, loadRegFromStackSlot, and
storeRegToStackSlot().

Since you're adding new physregs, you also need to ensure that the proper
GPR pairs are reserved. See for example X86RegisterInfo::getReservedRegs().

+def Tuples2R : RegisterTuples<[gsub_0, gsub_1],
+                              [(add R0, R2, R4, R6, R8, R10),
+                               (add R1, R3, R5, R7, R9, R11)]>;
+
+// 2 consecutive R registers.
+def GPRPair : RegisterClass<"ARM", [untyped], 64, (add Tuples2R)> {
+  let Size = 64; // 2 x 32 bits, we have no predefined type of that size.
+}

Please make the register class contain all the register pairs that an
instruction like ldrd can encode. The unwanted register pairs should be
reserved because one of their components are reserved.


The rest of your patch seems to be in the right general direction. You have
a number of unmotivated changes to target independent code that definitely
need a proper explanation.

/jakob