[LLVMdev] [RFC PATCH 1/2] x86: Fix ModR/M byte output in 16-bit addressing mode

Eric Christopher echristo at gmail.com
Mon Dec 16 11:46:41 PST 2013


Hi David,

I'm catching up on email at the moment so I don't know if you've done this,
but patches should go to llvm-commits for review if you wouldn't mind.

Thanks!

-eric

On Thu Dec 12 2013 at 8:39:19 AM, David Woodhouse <dwmw2 at infradead.org>
wrote:

> This attempts to address http://llvm.org/bugs/show_bug.cgi?id=18220
> It also fixes a test which was requiring the *wrong* output.
>
> I'm relatively happy with this part, and it even solves most of the hard
> part of feature request for .code16 in bug 8684 — which was actually why
> I started prodding at this. But I could do with some help with the
> 16-bit signed relocation handling, which I've split into a subsequent
> patch.
>
> diff --git a/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
> b/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
> index 7952607..12a30cf 100644
> --- a/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
> +++ b/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
> @@ -402,6 +402,56 @@ void X86MCCodeEmitter::EmitMemModRMByte(const MCInst
> &MI, unsigned Op,
>
>    unsigned BaseRegNo = BaseReg ? GetX86RegNum(Base) : -1U;
>
> +  // 16-bit addressing forms of the ModR/M byte have a different encoding
> for
> +  // the R/M field and are far more limited in which registers can be
> used.
> +  if (Is16BitMemOperand(MI, Op)) {
> +    if (BaseReg) {
> +      // See Table 2-1 "16-Bit Addressing Forms with the ModR/M byte"
> +      static const int R16Table[] = { 0, 0, 0, 7, 0, 6, 4, 5 };
> +      unsigned RMfield = R16Table[BaseRegNo];
> +
> +      assert(RMfield && "invalid 16-bit base register");
> +
> +      if (IndexReg.getReg()) {
> +        unsigned IndexReg16 = R16Table[GetX86RegNum(IndexReg)];
> +
> +        // Must have one of SI,DI (4,5), and one of BP/BX (6,7)
> +        assert(((IndexReg16 ^ RMfield) & 2) &&
> +               "invalid 16-bit base/index register combination");
> +        assert(Scale.getImm() == 1 &&
> +               "invalid scale for 16-bit memory reference");
> +
> +        if (IndexReg16 & 2)
> +          RMfield = (RMfield & 1) | ((7 - IndexReg16) << 1);
> +        else
> +          RMfield = (IndexReg16 & 1) | ((7 - RMfield) << 1);
> +      }
> +
> +      if (Disp.isImm() && isDisp8(Disp.getImm())) {
> +        // Use [REG]+disp8 form if we can, and for [BP] which cannot be
> encoded.
> +        if (BaseRegNo == N86::EBP || Disp.getImm() != 0) {
> +          EmitByte(ModRMByte(1, RegOpcodeField, RMfield), CurByte, OS);
> +          EmitImmediate(Disp, MI.getLoc(), 1, FK_Data_1, CurByte, OS,
> Fixups);
> +          return;
> +        } else {
> +          // No displacement
> +          EmitByte(ModRMByte(0, RegOpcodeField, RMfield), CurByte, OS);
> +          return;
> +        }
> +      }
> +      EmitByte(ModRMByte(2, RegOpcodeField, RMfield), CurByte, OS);
> +    } else {
> +      // !BaseReg
> +      EmitByte(ModRMByte(0, RegOpcodeField, 6), CurByte, OS);
> +    }
> +
> +    // FIXME: Yes we can!
> +    assert(Disp.isImm() && "cannot emit 16-bit relocation");
> +    // Emit 16-bit displacement for plain disp16 or [REG]+disp16 cases.
> +    EmitImmediate(Disp, MI.getLoc(), 2, FK_Data_2, CurByte, OS, Fixups);
> +    return;
> +  }
> +
>    // Determine whether a SIB byte is needed.
>    // If no BaseReg, issue a RIP relative instruction only if the MCE can
>    // resolve addresses on-the-fly, otherwise use SIB (Intel Manual 2A,
> table
> diff --git a/test/MC/X86/address-size.s b/test/MC/X86/address-size.s
> index b105b40..7b0bf6b 100644
> --- a/test/MC/X86/address-size.s
> +++ b/test/MC/X86/address-size.s
> @@ -8,6 +8,6 @@
>
>         .code32
>         movb    $0x0, (%si)
> -// CHECK: encoding: [0x67,0xc6,0x06,0x00]
> +// CHECK: encoding: [0x67,0xc6,0x04,0x00]
>         movb    $0x0, (%esi)
>  // CHECK: encoding: [0xc6,0x06,0x00]
> --
> 1.8.3.1
>
>
> --
>                    Sent with MeeGo's ActiveSync support.
>
> David Woodhouse                            Open Source Technology Centre
> David.Woodhouse at intel.com                              Intel Corporation
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131216/bb66ceca/attachment.html>


More information about the llvm-dev mailing list