[LLVMdev] Problems with 64-bit register operands of inline asm on ARM

[Weiming Zhao]

Hi  Måns,
Always forcing 64-bit operands into even/odd pairs may lead to subpoptimal
register allocation because not all 64 bit data requires paired regs. It
seems there is no general pattern to match. Maybe we should treat it case by
case.

Jakob, do you have any suggestions?

Thanks,
Weiming


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


-----Original Message-----
From: Måns Rullgård [mailto:mans at mansr.com] 
Sent: Wednesday, March 13, 2013 6:43 AM
To: llvmdev at cs.uiuc.edu
Cc: weimingz at codeaurora.org; renato.golin at linaro.org
Subject: Problems with 64-bit register operands of inline asm on ARM

r175088 attempted to fix gcc inline asm compatibility with 64-bit operands
by forcing these into even/odd register pairs the same way gcc always
allocates such values.

While the fix appears to work as such, it is not always activated when
required.  The patch makes the assumption that any inline asm statement
relying on the even/odd allocation will make use of the %Hn syntax to
reference the high (odd) register.  However, this is not the case.

A common pattern where llvm still fails is code using the abbreviated syntax
for LDRD and friends supported by gas:

    __asm__ ("ldrd %0, [%1]" : "=r"(a) : "r"(b));

Here the second destination register is implicitly one higher than the
first.  Because of this, the %H0 construct is never used, so the forced
even/odd allocation is skipped.

One possible fix, which I have tested, is to look for the specific
instructions requiring such a pair (LDRD/STRD and LDREXD/STREXD) in addition
to the 'H' modifier.  However, there are probably other creative ways in
which inline asm might rely on the specific pairing.
Thus I believe the safest solution is to always force 64-bit operands into
even/odd pairs for any inline asm.  In other words, we should probably do
something like this (untested):

--- a/lib/Target/ARM/ARMISelDAGToDAG.cpp
+++ b/lib/Target/ARM/ARMISelDAGToDAG.cpp
@@ -3457,19 +3457,6 @@ SDNode *ARMDAGToDAGISel::SelectInlineAsm(SDNode *N){
   bool Changed = false;
   unsigned NumOps = N->getNumOperands();
 
-  ExternalSymbolSDNode *S = dyn_cast<ExternalSymbolSDNode>(
-      N->getOperand(InlineAsm::Op_AsmString));
-  StringRef AsmString = StringRef(S->getSymbol());
-
-  // Normally, i64 data is bounded to two arbitrary GRPs for "%r"
constraint.
-  // However, some instrstions (e.g. ldrexd/strexd in ARM mode) require
-  // (even/even+1) GPRs and use %n and %Hn to refer to the individual regs
-  // respectively. Since there is no constraint to explicitly specify a
-  // reg pair, we search %H operand inside the asm string. If it is found,
the
-  // transformation below enforces a GPRPair reg class for "%r" for 64-bit
data.
-  if (AsmString.find(":H}") == StringRef::npos)
-    return NULL;
-
   DebugLoc dl = N->getDebugLoc();
   SDValue Glue = N->getOperand(NumOps-1);
 


--
Måns Rullgård
mans at mansr.com