[MC] Relax .fill size requirements

Avoid requirement that number of values must be known at assembler time. Fixes PR33586. Reviewers: rnk, peter.smith, echristo, jyknight Subscribers: hiraditya, llvm-commits Differential Revision: https://reviews.llvm.org/D46703 llvm-svn: 332741
llvm · May 18, 2018 · 588fad4 · 588fad4
1 parent fdde9f3
commit 588fad4
Show file tree

Hide file tree

Showing 9 changed files with 158 additions and 51 deletions.
diff --git a/llvm/include/llvm/MC/MCFragment.h b/llvm/include/llvm/MC/MCFragment.h
@@ -419,22 +419,23 @@ class MCPaddingFragment : public MCFragment {
 
 class MCFillFragment : public MCFragment {
   /// Value to use for filling bytes.
-  uint8_t Value;
-
+  uint64_t Value;
+  uint8_t ValueSize;
   /// The number of bytes to insert.
-  const MCExpr &Size;
+  const MCExpr &NumValues;
 
   /// Source location of the directive that this fragment was created for.
   SMLoc Loc;
 
 public:
-  MCFillFragment(uint8_t Value, const MCExpr &Size, SMLoc Loc,
-                 MCSection *Sec = nullptr)
-      : MCFragment(FT_Fill, false, 0, Sec), Value(Value), Size(Size), Loc(Loc) {
-  }
-
-  uint8_t getValue() const { return Value; }
-  const MCExpr &getSize() const { return Size; }
+  MCFillFragment(uint64_t Value, uint8_t VSize, const MCExpr &NumValues,
+                 SMLoc Loc, MCSection *Sec = nullptr)
+      : MCFragment(FT_Fill, false, 0, Sec), Value(Value), ValueSize(VSize),
+        NumValues(NumValues), Loc(Loc) {}
+
+  uint64_t getValue() const { return Value; }
+  uint8_t getValueSize() const { return ValueSize; }
+  const MCExpr &getNumValues() const { return NumValues; }
 
   SMLoc getLoc() const { return Loc; }
 

diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
@@ -295,10 +295,13 @@ uint64_t MCAssembler::computeFragmentSize(const MCAsmLayout &Layout,
     return cast<MCCompactEncodedInstFragment>(F).getContents().size();
   case MCFragment::FT_Fill: {
     auto &FF = cast<MCFillFragment>(F);
-    int64_t Size = 0;
-    if (!FF.getSize().evaluateAsAbsolute(Size, Layout))
+    int64_t NumValues = 0;
+    if (!FF.getNumValues().evaluateAsAbsolute(NumValues, Layout)) {
       getContext().reportError(FF.getLoc(),
                                "expected assembly-time absolute expression");
+      return 0;
+    }
+    int64_t Size = NumValues * FF.getValueSize();
     if (Size < 0) {
       getContext().reportError(FF.getLoc(), "invalid number of bytes");
       return 0;
@@ -557,19 +560,35 @@ static void writeFragment(const MCAssembler &Asm, const MCAsmLayout &Layout,
   case MCFragment::FT_Fill: {
     ++stats::EmittedFillFragments;
     const MCFillFragment &FF = cast<MCFillFragment>(F);
-    uint8_t V = FF.getValue();
+    uint64_t V = FF.getValue();
+    unsigned VSize = FF.getValueSize();
     const unsigned MaxChunkSize = 16;
     char Data[MaxChunkSize];
-    memcpy(Data, &V, 1);
-    for (unsigned I = 1; I < MaxChunkSize; ++I)
-      Data[I] = Data[0];
-
-    uint64_t Size = FragmentSize;
-    for (unsigned ChunkSize = MaxChunkSize; ChunkSize; ChunkSize /= 2) {
-      StringRef Ref(Data, ChunkSize);
-      for (uint64_t I = 0, E = Size / ChunkSize; I != E; ++I)
-        OW->writeBytes(Ref);
-      Size = Size % ChunkSize;
+    // Duplicate V into Data as byte vector to reduce number of
+    // writes done. As such, do endian conversion here, not in OW.
+    const bool isLittleEndian = Asm.getContext().getAsmInfo()->isLittleEndian();
+    for (unsigned I = 0; I != VSize; ++I) {
+      unsigned index = isLittleEndian ? I : (VSize - I - 1);
+      Data[I] = uint8_t(V >> (index * 8));
+    }
+    for (unsigned I = VSize; I < MaxChunkSize; ++I)
+      Data[I] = Data[I - VSize];
+
+    // Set to largest multiple of VSize in Data.
+    const unsigned NumPerChunk = MaxChunkSize / VSize;
+    // Set ChunkSize to largest multiple of VSize in Data
+    const unsigned ChunkSize = VSize * NumPerChunk;
+
+    // Do copies by chunk.
+    StringRef Ref(Data, ChunkSize);
+    for (uint64_t I = 0, E = FragmentSize / ChunkSize; I != E; ++I)
+      OW->writeBytes(Ref);
+
+    // do remainder if needed.
+    unsigned TrailingCount = FragmentSize % ChunkSize;
+    if (TrailingCount) {
+      StringRef RefTail(Data, TrailingCount);
+      OW->writeBytes(RefTail);
     }
     break;
   }

diff --git a/llvm/lib/MC/MCFragment.cpp b/llvm/lib/MC/MCFragment.cpp
@@ -388,7 +388,8 @@ LLVM_DUMP_METHOD void MCFragment::dump() const {
   case MCFragment::FT_Fill:  {
     const MCFillFragment *FF = cast<MCFillFragment>(this);
     OS << " Value:" << static_cast<unsigned>(FF->getValue())
-       << " Size:" << FF->getSize();
+       << " ValueSize:" << static_cast<unsigned>(FF->getValueSize())
+       << " NumValues:" << FF->getNumValues();
     break;
   }
   case MCFragment::FT_Relaxable:  {

diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -637,31 +637,37 @@ void MCObjectStreamer::emitFill(const MCExpr &NumBytes, uint64_t FillValue,
   flushPendingLabels(DF, DF->getContents().size());
 
   assert(getCurrentSectionOnly() && "need a section");
-  insert(new MCFillFragment(FillValue, NumBytes, Loc));
+  insert(new MCFillFragment(FillValue, 1, NumBytes, Loc));
 }
 
 void MCObjectStreamer::emitFill(const MCExpr &NumValues, int64_t Size,
                                 int64_t Expr, SMLoc Loc) {
   int64_t IntNumValues;
-  if (!NumValues.evaluateAsAbsolute(IntNumValues, getAssemblerPtr())) {
-    getContext().reportError(Loc, "expected absolute expression");
+  // Do additional checking now if we can resolve the value.
+  if (NumValues.evaluateAsAbsolute(IntNumValues, getAssemblerPtr())) {
+    if (IntNumValues < 0) {
+      getContext().getSourceManager()->PrintMessage(
+          Loc, SourceMgr::DK_Warning,
+          "'.fill' directive with negative repeat count has no effect");
+      return;
+    }
+    // Emit now if we can for better errors.
+    int64_t NonZeroSize = Size > 4 ? 4 : Size;
+    Expr &= ~0ULL >> (64 - NonZeroSize * 8);
+    for (uint64_t i = 0, e = IntNumValues; i != e; ++i) {
+      EmitIntValue(Expr, NonZeroSize);
+      if (NonZeroSize < Size)
+        EmitIntValue(0, Size - NonZeroSize);
+    }
     return;
   }
 
-  if (IntNumValues < 0) {
-    getContext().getSourceManager()->PrintMessage(
-        Loc, SourceMgr::DK_Warning,
-        "'.fill' directive with negative repeat count has no effect");
-    return;
-  }
+  // Otherwise emit as fragment.
+  MCDataFragment *DF = getOrCreateDataFragment();
+  flushPendingLabels(DF, DF->getContents().size());
 
-  int64_t NonZeroSize = Size > 4 ? 4 : Size;
-  Expr &= ~0ULL >> (64 - NonZeroSize * 8);
-  for (uint64_t i = 0, e = IntNumValues; i != e; ++i) {
-    EmitIntValue(Expr, NonZeroSize);
-    if (NonZeroSize < Size)
-      EmitIntValue(0, Size - NonZeroSize);
-  }
+  assert(getCurrentSectionOnly() && "need a section");
+  insert(new MCFillFragment(Expr, Size, NumValues, Loc));
 }
 
 void MCObjectStreamer::EmitFileDirective(StringRef Filename) {

diff --git a/llvm/lib/MC/WasmObjectWriter.cpp b/llvm/lib/MC/WasmObjectWriter.cpp
@@ -630,10 +630,11 @@ static void addData(SmallVectorImpl<char> &DataBytes,
                                              Align->getMaxBytesToEmit());
       DataBytes.resize(Size, Value);
     } else if (auto *Fill = dyn_cast<MCFillFragment>(&Frag)) {
-      int64_t Size;
-      if (!Fill->getSize().evaluateAsAbsolute(Size))
+      int64_t NumValues;
+      if (!Fill->getNumValues().evaluateAsAbsolute(NumValues))
         llvm_unreachable("The fill should be an assembler constant");
-      DataBytes.insert(DataBytes.end(), Size, Fill->getValue());
+      DataBytes.insert(DataBytes.end(), Fill->getValueSize() * NumValues,
+                       Fill->getValue());
     } else {
       const auto &DataFrag = cast<MCDataFragment>(Frag);
       const SmallVectorImpl<char> &Contents = DataFrag.getContents();

diff --git a/llvm/test/MC/ARM/assembler-fill.s b/llvm/test/MC/ARM/assembler-fill.s
@@ -0,0 +1,23 @@
+// RUN: llvm-mc --triple=thumbv7eb-linux-gnueabihf %s -filetype=obj | llvm-objdump -triple=thumbv7eb-linux-gnueabihf -s - | FileCheck %s
+
+// CHECK: Contents of section .text
+// CHECK-NEXT: 0000 d000bf00 
+
+//  Make sure we emit in correct endianness.
+
+// CHECK: Contents of section .data
+// CHECK-NEXT:  0000 12341234 1234 
+
+	.syntax unified
+        .text
+        .thumb
+	.thumb_func
+.L1:
+        beq Label
+.L2:
+	nop
+Label:
+
+	.data
+	.short 0x1234
+	.fill (.L2 - .L1), 2, 0x1234
diff --git a/llvm/test/MC/AsmParser/assembler-expressions.s b/llvm/test/MC/AsmParser/assembler-expressions.s
@@ -28,7 +28,8 @@ foo3:
 # nop is a fixed size instruction so this should pass.
 
 # OBJTEXT: Contents of section .text
-# OBJTEXT-NEXT: 0000 9090ff34 25
+# OBJTEXT-NEXT: 0000 9090ff34 25000000 00909090 78563412
+# OBJTEXT-NEXT: 0010 78563412 90
 
 .text
 
@@ -41,7 +42,23 @@ text1:
 	ret
 .endif
 	push gs
-
+	nop
+	nop
+	nop
 # No additional errors.
 #	
 # ASM-ERR-NOT: {{[0-9]+}}:{{[0-9]+}}: error:
+
+
+
+text2:
+	.long 0x12345678
+text3:
+	.fill (text3-text2)/4, 4, 0x12345678
+	nop
+
+
+
+
+
+
diff --git a/llvm/test/MC/AsmParser/directive_fill.s b/llvm/test/MC/AsmParser/directive_fill.s
@@ -1,7 +1,7 @@
 # RUN: llvm-mc -triple i386-unknown-unknown %s 2> %t.err | FileCheck %s
 # RUN: FileCheck --check-prefix=CHECK-WARNINGS %s < %t.err
-# RUN: llvm-mc -triple i386-unknown-unknown -filetype=obj -o %t.o %s 2> %t.err2
-# RUN: FileCheck --check-prefix=OBJ-WARNINGS %s < %t.err2
+# RUN: not llvm-mc -triple i386-unknown-unknown -filetype=obj -o %t.o %s 2> %t.err2
+# RUN: FileCheck --check-prefix=OBJ-ERRS %s < %t.err2
 
 # CHECK: TEST0:
 # CHECK: .fill 1, 1, 0xa
@@ -47,7 +47,7 @@ TEST7:
 
 # CHECK: TEST8
 # CHECK: .fill -1, 8, 0x1
-# OBJ-WARNINGS: '.fill' directive with negative repeat count has no effect
+# OBJ-ERRS: '.fill' directive with negative repeat count has no effect
 TEST8:
 	.fill -1, 8, 1
 
@@ -66,7 +66,14 @@ TEST11:
   .fill TEST11 - TEST10
 
 # CHECK: TEST12
-# CHECK: .fill TEST11-TEST12, 3, 0x12345678
-# OBJ-WARNINGS: '.fill' directive with negative repeat count has no effect
+# CHECK: .fill TEST11-TEST12, 4, 0x12345678
+# OBJ-ERRS: '.fill' directive with negative repeat count has no effect
 TEST12:
-  .fill TEST11 - TEST12, 3, 0x12345678
+	.fill TEST11 - TEST12, 4, 0x12345678
+
+# CHECK: TEST13
+# CHECK: .fill (TEST11-TEST12)+i, 4, 0x12345678
+# OBJ-ERRS: [[@LINE+2]]:8: error: expected assembly-time absolute expression
+TEST13:
+	.fill TEST11 - TEST12+i, 4, 0x12345678
+
diff --git a/llvm/test/MC/AsmParser/directive_fill_2.s b/llvm/test/MC/AsmParser/directive_fill_2.s
@@ -0,0 +1,32 @@
+# RUN: llvm-mc -filetype=obj -defsym=I=6 -triple i686-unknown-unknown %s | llvm-objdump -triple i686-unknown-unknown -s - | FileCheck --check-prefix="CHECK" %s
+# RUN: not llvm-mc -filetype=obj -defsym=I=4 -triple i686-unknown-unknown %s -o /dev/null 2>&1 | FileCheck --check-prefix="CHECK-ERR" %s
+
+
+
+# CHECK: Contents of section .text
+# CHECK-NEXT: 0000 e9810000 00cc9090 90909090 90909090
+
+# Make sure we emit in correct endianness.
+
+# CHECK: Contents of section .data
+# CHECK-NEXT: 0000 78563412 78563412 78563412
+
+.text
+foo:
+jmp bar2
+# CHECK-ERR: [[@LINE+1]]:7: error: invalid number of bytes
+.fill ((I+foo) - .), 1, 0xcc
+bar:
+ .space 128, 0x90
+bar2:
+.byte 0xff
+
+# This fill length is not known at assembler time.
+
+.if (I==6)
+
+.data
+.long 0x12345678
+.fill ((foo+8)-bar), 4, 0x12345678
+
+.endif