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

[David Woodhouse]

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



-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5745 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131212/e78fa462/attachment.bin>