[llvm-commits] [llvm] r121336 - in /llvm/trunk: lib/Target/ARM/ARMAsmBackend.cpp lib/Target/ARM/ARMCodeEmitter.cpp lib/Target/ARM/ARMFixupKinds.h lib/Target/ARM/ARMInstrThumb.td lib/Target/ARM/ARMMCCodeEmitter.cpp utils/TableGen/EDEmitter.cpp

Jim Grosbach grosbach at apple.com
Thu Dec 9 11:35:12 PST 2010


Hi Bill,

You're analysis of what's wrong with the value is right on, but I'm not sure this is the right way to solve it. Specifically, the fixup value we're encoding into these instructions should be 32-bit aligned, and thus have that bit as zero already. That it's not happening that way indicates something is wrong further upstream. Owen ran into this with some ldr instructions as well, for example. 8745346.

-Jim

On Dec 8, 2010, at 4:39 PM, Bill Wendling wrote:

> Author: void
> Date: Wed Dec  8 18:39:08 2010
> New Revision: 121336
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=121336&view=rev
> Log:
> The BLX instruction is encoded differently than the BL, because why not? In
> particular, the immediate has 20-bits of value instead of 21. And bit 0 is '0'
> always. Going through the BL fixup encoding was trashing the "bit 0 is '0'"
> invariant.
> 
> Attempt to get the encoding at slightly more correct with this.
> 
> Modified:
>    llvm/trunk/lib/Target/ARM/ARMAsmBackend.cpp
>    llvm/trunk/lib/Target/ARM/ARMCodeEmitter.cpp
>    llvm/trunk/lib/Target/ARM/ARMFixupKinds.h
>    llvm/trunk/lib/Target/ARM/ARMInstrThumb.td
>    llvm/trunk/lib/Target/ARM/ARMMCCodeEmitter.cpp
>    llvm/trunk/utils/TableGen/EDEmitter.cpp
> 
> Modified: llvm/trunk/lib/Target/ARM/ARMAsmBackend.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMAsmBackend.cpp?rev=121336&r1=121335&r2=121336&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMAsmBackend.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMAsmBackend.cpp Wed Dec  8 18:39:08 2010
> @@ -146,13 +146,33 @@
>     // The value doesn't encode the low bit (always zero) and is offset by
>     // four. The value is encoded into disjoint bit positions in the destination
>     // opcode. x = unchanged, I = immediate value bit, S = sign extension bit
> -    // xxxxxSIIIIIIIIII xxxxxIIIIIIIIIII
> +    // 
> +    //   BL:  xxxxxSIIIIIIIIII xxxxxIIIIIIIIIII
> +    // 
>     // Note that the halfwords are stored high first, low second; so we need
>     // to transpose the fixup value here to map properly.
> -    // FIXME: Something isn't quite right with this. Some, but not all, BLX
> -    // instructions are getting the encoded value off by one.
> +    unsigned isNeg = (int64_t(Value) < 0) ? 1 : 0;
>     uint32_t Binary = 0x3fffff & ((Value - 4) >> 1);
> -    Binary = ((Binary & 0x7ff) << 16) | (Binary >> 11);
> +    Binary  = (Binary & 0x7ff) << 16;    // Low imm11 value.
> +    Binary |= (Binary & 0x1ffc00) >> 11; // High imm10 value.
> +    Binary |= isNeg << 10;               // Sign bit.
> +    return Binary;
> +  }
> +  case ARM::fixup_arm_thumb_blx: {
> +    // The value doesn't encode the low two bits (always zero) and is offset by
> +    // four (see fixup_arm_thumb_cp). The value is encoded into disjoint bit
> +    // positions in the destination opcode. x = unchanged, I = immediate value
> +    // bit, S = sign extension bit, 0 = zero.
> +    // 
> +    //   BLX: xxxxxSIIIIIIIIII xxxxxIIIIIIIIII0
> +    // 
> +    // Note that the halfwords are stored high first, low second; so we need
> +    // to transpose the fixup value here to map properly.
> +    unsigned isNeg = (int64_t(Value) < 0) ? 1 : 0;
> +    uint32_t Binary = 0xfffff & ((Value - 2) >> 2);
> +    Binary  = (Binary & 0x3ff) << 17;    // Low imm10L value.
> +    Binary |= (Binary & 0xffc00) >> 10;  // High imm10H value.
> +    Binary |= isNeg << 10;               // Sign bit.
>     return Binary;
>   }
>   case ARM::fixup_arm_thumb_cp:
> @@ -291,6 +311,7 @@
>   case ARM::fixup_t2_branch:
>   case ARM::fixup_t2_pcrel_10:
>   case ARM::fixup_arm_thumb_bl:
> +  case ARM::fixup_arm_thumb_blx:
>     return 4;
>   }
> }
> 
> Modified: llvm/trunk/lib/Target/ARM/ARMCodeEmitter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMCodeEmitter.cpp?rev=121336&r1=121335&r2=121336&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMCodeEmitter.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMCodeEmitter.cpp Wed Dec  8 18:39:08 2010
> @@ -173,6 +173,8 @@
>       const { return 0; }
>     unsigned getThumbBLTargetOpValue(const MachineInstr &MI, unsigned Op)
>       const { return 0; }
> +    unsigned getThumbBLXTargetOpValue(const MachineInstr &MI, unsigned Op)
> +      const { return 0; }
>     unsigned getThumbBRTargetOpValue(const MachineInstr &MI, unsigned Op)
>       const { return 0; }
>     unsigned getBranchTargetOpValue(const MachineInstr &MI, unsigned Op)
> 
> Modified: llvm/trunk/lib/Target/ARM/ARMFixupKinds.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMFixupKinds.h?rev=121336&r1=121335&r2=121336&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMFixupKinds.h (original)
> +++ llvm/trunk/lib/Target/ARM/ARMFixupKinds.h Wed Dec  8 18:39:08 2010
> @@ -35,9 +35,12 @@
>   // instructions.
>   fixup_t2_branch,
> 
> -  // fixup_arm_thumb_bl - Fixup for Thumb BL/BLX instructions.
> +  // fixup_arm_thumb_blx - Fixup for Thumb BL instructions.
>   fixup_arm_thumb_bl,
> 
> +  // fixup_arm_thumb_blx - Fixup for Thumb BLX instructions.
> +  fixup_arm_thumb_blx,
> +
>   // fixup_arm_thumb_br - Fixup for Thumb branch instructions.
>   fixup_arm_thumb_br,
> 
> 
> Modified: llvm/trunk/lib/Target/ARM/ARMInstrThumb.td
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMInstrThumb.td?rev=121336&r1=121335&r2=121336&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMInstrThumb.td (original)
> +++ llvm/trunk/lib/Target/ARM/ARMInstrThumb.td Wed Dec  8 18:39:08 2010
> @@ -82,6 +82,10 @@
>   let EncoderMethod = "getThumbBLTargetOpValue";
> }
> 
> +def t_blxtarget : Operand<i32> {
> +  let EncoderMethod = "getThumbBLXTargetOpValue";
> +}
> +
> def MemModeThumbAsmOperand : AsmOperandClass {
>   let Name = "MemModeThumb";
>   let SuperClasses = [];
> @@ -395,7 +399,7 @@
> 
>   // ARMv5T and above, also used for Thumb2
>   def tBLXi : TIx2<0b11110, 0b11, 0,
> -                   (outs), (ins t_bltarget:$func, variable_ops), IIC_Br,
> +                   (outs), (ins t_blxtarget:$func, variable_ops), IIC_Br,
>                    "blx\t$func",
>                    [(ARMcall tglobaladdr:$func)]>,
>               Requires<[IsThumb, HasV5T, IsNotDarwin]> {
> @@ -448,7 +452,7 @@
> 
>   // ARMv5T and above, also used for Thumb2
>   def tBLXi_r9 : TIx2<0b11110, 0b11, 0,
> -                      (outs), (ins pred:$p, t_bltarget:$func, variable_ops),
> +                      (outs), (ins pred:$p, t_blxtarget:$func, variable_ops),
>                       IIC_Br, "blx${p}\t$func",
>                       [(ARMcall tglobaladdr:$func)]>,
>                  Requires<[IsThumb, HasV5T, IsDarwin]> {
> 
> Modified: llvm/trunk/lib/Target/ARM/ARMMCCodeEmitter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMMCCodeEmitter.cpp?rev=121336&r1=121335&r2=121336&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMMCCodeEmitter.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMMCCodeEmitter.cpp Wed Dec  8 18:39:08 2010
> @@ -53,6 +53,7 @@
>       { "fixup_arm_branch",         1,    24,   MCFixupKindInfo::FKF_IsPCRel },
>       { "fixup_t2_branch",          0,    32,   MCFixupKindInfo::FKF_IsPCRel },
>       { "fixup_arm_thumb_bl",       0,    32,   MCFixupKindInfo::FKF_IsPCRel },
> +      { "fixup_arm_thumb_blx",      0,    32,   MCFixupKindInfo::FKF_IsPCRel },
>       { "fixup_arm_thumb_br",       0,    16,   MCFixupKindInfo::FKF_IsPCRel },
>       { "fixup_arm_thumb_cp",       1,    8,    MCFixupKindInfo::FKF_IsPCRel },
>       { "fixup_arm_movt_hi16",      0,    16,   0 },
> @@ -87,10 +88,15 @@
>                               SmallVectorImpl<MCFixup> &Fixups) const;
> 
>   /// getThumbBLTargetOpValue - Return encoding info for Thumb immediate
> -  /// branch target.
> +  /// BL branch target.
>   uint32_t getThumbBLTargetOpValue(const MCInst &MI, unsigned OpIdx,
>                                    SmallVectorImpl<MCFixup> &Fixups) const;
> 
> +  /// getThumbBLXTargetOpValue - Return encoding info for Thumb immediate
> +  /// BLX branch target.
> +  uint32_t getThumbBLXTargetOpValue(const MCInst &MI, unsigned OpIdx,
> +                                    SmallVectorImpl<MCFixup> &Fixups) const;
> +
>   /// getThumbBRTargetOpValue - Return encoding info for Thumb branch target.
>   uint32_t getThumbBRTargetOpValue(const MCInst &MI, unsigned OpIdx,
>                                    SmallVectorImpl<MCFixup> &Fixups) const;
> @@ -443,6 +449,14 @@
>   return ::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_arm_thumb_bl, Fixups);
> }
> 
> +/// getThumbBLXTargetOpValue - Return encoding info for Thumb immediate
> +/// BLX branch target.
> +uint32_t ARMMCCodeEmitter::
> +getThumbBLXTargetOpValue(const MCInst &MI, unsigned OpIdx,
> +                         SmallVectorImpl<MCFixup> &Fixups) const {
> +  return ::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_arm_thumb_blx, Fixups);
> +}
> +
> /// getThumbBRTargetOpValue - Return encoding info for Thumb branch target.
> uint32_t ARMMCCodeEmitter::
> getThumbBRTargetOpValue(const MCInst &MI, unsigned OpIdx,
> @@ -740,17 +754,7 @@
> uint32_t ARMMCCodeEmitter::
> getAddrModePCOpValue(const MCInst &MI, unsigned OpIdx,
>                      SmallVectorImpl<MCFixup> &Fixups) const {
> -  const MCOperand &MO = MI.getOperand(OpIdx);
> -
> -  // If the destination is an immediate, we have nothing to do.
> -  if (MO.isImm()) return MO.getImm();
> -  assert (MO.isExpr() && "Unexpected branch target type!");
> -  const MCExpr *Expr = MO.getExpr();
> -  MCFixupKind Kind = MCFixupKind(ARM::fixup_arm_thumb_cp);
> -  Fixups.push_back(MCFixup::Create(0, Expr, Kind));
> -
> -  // All of the information is in the fixup.
> -  return 0;
> +  return ::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_arm_thumb_cp, Fixups);
> }
> 
> /// getAddrMode5OpValue - Return encoding info for 'reg +/- imm10' operand.
> 
> Modified: llvm/trunk/utils/TableGen/EDEmitter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/EDEmitter.cpp?rev=121336&r1=121335&r2=121336&view=diff
> ==============================================================================
> --- llvm/trunk/utils/TableGen/EDEmitter.cpp (original)
> +++ llvm/trunk/utils/TableGen/EDEmitter.cpp Wed Dec  8 18:39:08 2010
> @@ -590,6 +590,7 @@
>   MISC("t_brtarget", "kOperandTypeARMBranchTarget");              // ?
>   MISC("bltarget", "kOperandTypeARMBranchTarget");                // ?
>   MISC("t_bltarget", "kOperandTypeARMBranchTarget");              // ?
> +  MISC("t_blxtarget", "kOperandTypeARMBranchTarget");             // ?
>   MISC("so_reg", "kOperandTypeARMSoReg");                         // R, R, I
>   MISC("shift_so_reg", "kOperandTypeARMSoReg");                   // R, R, I
>   MISC("t2_so_reg", "kOperandTypeThumb2SoReg");                   // R, I
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits





More information about the llvm-commits mailing list