[llvm] r194024 - AArch64: use default asm operand printing when modifier inapplicable

Richard Barton richard.barton at arm.com
Fri Nov 8 10:05:39 PST 2013


Hi Tim

After this patch, the test in CodeGen/Generic/asm-large-immediate.ll fails
for aarch64 because the AArch64 PrintAsmOperand no longer handles the "n"
operand modifier properly. 

The new PrintAsmOperand no longer calls out to the generic AsmPrinter for
InlineAsm which used to handle this modifier. The attached patch calls out
to the superclass again and falls through if there is no match. I have also
removed the redundant case for 'c' as this gets handled by the superclass
too.

Happy for me to commit this one?

Rich


> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> bounces at cs.uiuc.edu] On Behalf Of Tim Northover
> Sent: 04 November 2013 23:04
> To: llvm-commits at cs.uiuc.edu
> Subject: [llvm] r194024 - AArch64: use default asm operand printing when
> modifier inapplicable
> 
> Author: tnorthover
> Date: Mon Nov  4 17:04:07 2013
> New Revision: 194024
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=194024&view=rev
> Log:
> AArch64: use default asm operand printing when modifier inapplicable
> 
> If an inline assembly operand has multiple constraints (e.g. "Ir" for
immediate or
> register) and an operand modifier (E.g. "w" for "print register as wN")
then we
> need to decide behaviour when the modifier doesn't apply to the
constraint.
> 
> Previousely produced some combination of an assertion failure and a fatal
error.
> GCC's behaviour appears to be to ignore the modifier and print the operand
in
> the default way. This patch should implement that.
> 
> Modified:
>     llvm/trunk/lib/Target/AArch64/AArch64AsmPrinter.cpp
>     llvm/trunk/test/CodeGen/AArch64/inline-asm-modifiers.ll
> 
> Modified: llvm/trunk/lib/Target/AArch64/AArch64AsmPrinter.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/Target/AArch64/AArch64AsmPrinter.cpp?rev=194024&r1
> =194023&r2=194024&view=diff
> =================================================================
> =============
> --- llvm/trunk/lib/Target/AArch64/AArch64AsmPrinter.cpp (original)
> +++ llvm/trunk/lib/Target/AArch64/AArch64AsmPrinter.cpp Mon Nov  4
> +++ 17:04:07 2013
> @@ -82,7 +82,7 @@ bool AArch64AsmPrinter::printSymbolicAdd
>    StringRef Modifier;
>    switch (MO.getType()) {
>    default:
> -    llvm_unreachable("Unexpected operand for symbolic address
constraint");
> +    return true;
>    case MachineOperand::MO_GlobalAddress:
>      Name = getSymbol(MO.getGlobal())->getName();
> 
> @@ -146,57 +146,33 @@ bool AArch64AsmPrinter::PrintAsmOperand(
>                                          unsigned AsmVariant,
>                                          const char *ExtraCode,
raw_ostream &O) {
>    const TargetRegisterInfo *TRI = MF->getTarget().getRegisterInfo();
> -  if (!ExtraCode || !ExtraCode[0]) {
> -    // There's actually no operand modifier, which leads to a slightly
eclectic
> -    // set of behaviour which we have to handle here.
> -    const MachineOperand &MO = MI->getOperand(OpNum);
> -    switch (MO.getType()) {
> -    default:
> -      llvm_unreachable("Unexpected operand for inline assembly");
> -    case MachineOperand::MO_Register:
> -      // GCC prints the unmodified operand of a 'w' constraint as the
vector
> -      // register. Technically, we could allocate the argument as a
VPR128, but
> -      // that leads to extremely dodgy copies being generated to get the
data
> -      // there.
> -      if (printModifiedFPRAsmOperand(MO, TRI, 'v', O))
> -        O << AArch64InstPrinter::getRegisterName(MO.getReg());
> -      break;
> -    case MachineOperand::MO_Immediate:
> -      O << '#' << MO.getImm();
> -      break;
> -    case MachineOperand::MO_FPImmediate:
> -      assert(MO.getFPImm()->isExactlyValue(0.0) && "Only FP 0.0
expected");
> -      O << "#0.0";
> -      break;
> -    case MachineOperand::MO_BlockAddress:
> -    case MachineOperand::MO_ConstantPoolIndex:
> -    case MachineOperand::MO_GlobalAddress:
> -    case MachineOperand::MO_ExternalSymbol:
> -      return printSymbolicAddress(MO, false, "", O);
> -    }
> -    return false;
> -  }
> 
> -  // We have a real modifier to handle.
> +  if (!ExtraCode)
> +    ExtraCode = "";
> +
>    switch(ExtraCode[0]) {
>    default:
> -    // See if this is a generic operand
> -    return AsmPrinter::PrintAsmOperand(MI, OpNum, AsmVariant, ExtraCode,
O);
> +    break;
>    case 'c': // Don't print "#" before an immediate operand.
> -    if (!MI->getOperand(OpNum).isImm())
> -      return true;
> -    O << MI->getOperand(OpNum).getImm();
> -    return false;
> +    if (MI->getOperand(OpNum).isImm()) {
> +      O << MI->getOperand(OpNum).getImm();
> +      return false;
> +    }
> +    break;
>    case 'w':
>      // Output 32-bit general register operand, constant zero as wzr, or
stack
>      // pointer as wsp. Ignored when used with other operand types.
> -    return printModifiedGPRAsmOperand(MI->getOperand(OpNum), TRI,
> -                                      AArch64::GPR32RegClass, O);
> +    if (!printModifiedGPRAsmOperand(MI->getOperand(OpNum), TRI,
> +                                    AArch64::GPR32RegClass, O))
> +      return false;
> +    break;
>    case 'x':
>      // Output 64-bit general register operand, constant zero as xzr, or
stack
>      // pointer as sp. Ignored when used with other operand types.
> -    return printModifiedGPRAsmOperand(MI->getOperand(OpNum), TRI,
> -                                      AArch64::GPR64RegClass, O);
> +    if (!printModifiedGPRAsmOperand(MI->getOperand(OpNum), TRI,
> +                                    AArch64::GPR64RegClass, O))
> +      return false;
> +    break;
>    case 'H':
>      // Output higher numbered of a 64-bit general register pair
>    case 'Q':
> @@ -216,25 +192,61 @@ bool AArch64AsmPrinter::PrintAsmOperand(
>    case 's':
>    case 'd':
>    case 'q':
> -    return printModifiedFPRAsmOperand(MI->getOperand(OpNum), TRI,
> -                                      ExtraCode[0], O);
> +    if (!printModifiedFPRAsmOperand(MI->getOperand(OpNum), TRI,
> +                                    ExtraCode[0], O))
> +      return false;
> +    break;
>    case 'A':
>      // Output symbolic address with appropriate relocation modifier (also
>      // suitable for ADRP).
> -    return printSymbolicAddress(MI->getOperand(OpNum), false, "", O);
> +    if (!printSymbolicAddress(MI->getOperand(OpNum), false, "", O))
> +      return false;
> +    break;
>    case 'L':
>      // Output bits 11:0 of symbolic address with appropriate :lo12:
relocation
>      // modifier.
> -    return printSymbolicAddress(MI->getOperand(OpNum), true, "lo12", O);
> +    if (!printSymbolicAddress(MI->getOperand(OpNum), true, "lo12", O))
> +      return false;
> +    break;
>    case 'G':
>      // Output bits 23:12 of symbolic address with appropriate :hi12:
relocation
>      // modifier (currently only for TLS local exec).
> -    return printSymbolicAddress(MI->getOperand(OpNum), true, "hi12", O);
> +    if (!printSymbolicAddress(MI->getOperand(OpNum), true, "hi12", O))
> +      return false;
> +    break;
>    case 'a':
>      return PrintAsmMemoryOperand(MI, OpNum, AsmVariant, ExtraCode, O);
>    }
> 
> +  // There's actually no operand modifier, which leads to a slightly
> + eclectic  // set of behaviour which we have to handle here.
> +  const MachineOperand &MO = MI->getOperand(OpNum);  switch
> + (MO.getType()) {
> +  default:
> +    llvm_unreachable("Unexpected operand for inline assembly");  case
> + MachineOperand::MO_Register:
> +    // GCC prints the unmodified operand of a 'w' constraint as the
vector
> +    // register. Technically, we could allocate the argument as a VPR128,
but
> +    // that leads to extremely dodgy copies being generated to get the
data
> +    // there.
> +    if (printModifiedFPRAsmOperand(MO, TRI, 'v', O))
> +      O << AArch64InstPrinter::getRegisterName(MO.getReg());
> +    break;
> +  case MachineOperand::MO_Immediate:
> +    O << '#' << MO.getImm();
> +    break;
> +  case MachineOperand::MO_FPImmediate:
> +    assert(MO.getFPImm()->isExactlyValue(0.0) && "Only FP 0.0 expected");
> +    O << "#0.0";
> +    break;
> +  case MachineOperand::MO_BlockAddress:
> +  case MachineOperand::MO_ConstantPoolIndex:
> +  case MachineOperand::MO_GlobalAddress:
> +  case MachineOperand::MO_ExternalSymbol:
> +    return printSymbolicAddress(MO, false, "", O);  }
> 
> +  return false;
>  }
> 
>  bool AArch64AsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI,
> 
> Modified: llvm/trunk/test/CodeGen/AArch64/inline-asm-modifiers.ll
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/CodeGen/AArch64/inline-asm-
> modifiers.ll?rev=194024&r1=194023&r2=194024&view=diff
> =================================================================
> =============
> --- llvm/trunk/test/CodeGen/AArch64/inline-asm-modifiers.ll (original)
> +++ llvm/trunk/test/CodeGen/AArch64/inline-asm-modifiers.ll Mon Nov  4
> +++ 17:04:07 2013
> @@ -22,6 +22,11 @@ define void @test_inline_modifier_L() no  ; CHECK: ldr
x0,
> [x0, #:gottprel_lo12:var_tlsie]  ; CHECK: add x0, x0,
#:tprel_lo12:var_tlsle
> 
> +  call void asm sideeffect "add x0, x0, ${0:L}", "Si,~{x0}"(i32 64)
> +  call void asm sideeffect "ldr x0, [x0, ${0:L}]", "Si,~{x0}"(i32 64) ;
> +CHECK: add x0, x0, #64 ; CHECK: ldr x0, [x0, #64]
> +
>    ret void
>  }
> 
> @@ -32,6 +37,8 @@ define void @test_inline_modifier_G() no  ; CHECK: add
x0,
> x0, #:dtprel_hi12:var_tlsld, lsl #12  ; CHECK: add x0, x0,
#:tprel_hi12:var_tlsle, lsl
> #12
> 
> +  call void asm sideeffect "add x0, x0, ${0:G}", "Si,~{x0}"(i32 42) ;
> +CHECK: add x0, x0, #42
>    ret void
>  }
> 
> @@ -47,6 +54,9 @@ define void @test_inline_modifier_A() no  ; CHECK: adrp
> x0, :tlsdesc:var_tlsgd  ; CHECK: adrp x0, :gottprel:var_tlsie
> 
> +  call void asm sideeffect "adrp x0, ${0:A}", "Si,~{x0}"(i32 40) ;
> +CHECK: adrp x0, #40
> +
>    ret void
>  }
> 
> @@ -71,6 +81,12 @@ define void @test_inline_modifier_wx(i32
>    call i32 asm sideeffect "add ${0:x}, ${1:x}, ${1:x}", "=r,r"(i32 0)  ;
CHECK: add
> {{w[0-9]+}}, wzr, wzr  ; CHECK: add {{x[0-9]+}}, xzr, xzr
> +
> +  call i32 asm sideeffect "add ${0:w}, ${0:w}, ${1:w}", "=r,Ir,0"(i32
> +123, i32 %small)
> +  call i64 asm sideeffect "add ${0:x}, ${0:x}, ${1:x}", "=r,Ir,0"(i32
> +456, i64 %big) ; CHECK: add {{w[0-9]+}}, {{w[0-9]+}}, #123 ; CHECK: add
> +{{x[0-9]+}}, {{x[0-9]+}}, #456
> +
>    ret void
>  }
> 
> @@ -97,6 +113,18 @@ define void @test_inline_modifier_bhsdq(  ; CHECK: ldr
> s0, [sp]  ; CHECK: ldr d0, [sp]  ; CHECK: ldr q0, [sp]
> +
> +  call void asm sideeffect "fcmp b0, ${0:b}", "Yw"(float 0.0)
> +  call void asm sideeffect "fcmp h0, ${0:h}", "Yw"(float 0.0)
> +  call void asm sideeffect "fcmp s0, ${0:s}", "Yw"(float 0.0)
> +  call void asm sideeffect "fcmp d0, ${0:d}", "Yw"(float 0.0)
> +  call void asm sideeffect "fcmp q0, ${0:q}", "Yw"(float 0.0) ; CHECK:
> +fcmp b0, #0 ; CHECK: fcmp h0, #0 ; CHECK: fcmp s0, #0 ; CHECK: fcmp d0,
> +#0 ; CHECK: fcmp q0, #0
> +
>    ret void
>  }
> 
> @@ -116,3 +144,4 @@ define void @test_inline_modifier_a() no  ; CHECK:
prfm
> pldl1keep, [x[[VARADDR]]]
>    ret void
>  }
> +
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
A non-text attachment was scrubbed...
Name: inline_asm_fix.patch
Type: application/octet-stream
Size: 758 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131108/b9281eb4/attachment.obj>


More information about the llvm-commits mailing list