[PATCH] [MC] Write padding into fragments when -mc-relax-all flag is used

Petr Hosek phosek at chromium.org
Mon Mar 9 17:35:24 PDT 2015


I have added the description of the proposed scheme to http://www.chromium.org/nativeclient/pnacl/aligned-bundling-support-in-llvm as requested.


================
Comment at: include/llvm/MC/MCAsmLayout.h:112
@@ +111,3 @@
+  /// obey bundling restrictions.
+  static uint64_t computeBundlePadding(const MCAssembler &Assembler,
+                                       const MCFragment *F,
----------------
eliben wrote:
> Is there a good reason to keep this a member and not just a standalone function?
Refactored as a standalone function.

================
Comment at: include/llvm/MC/MCAssembler.h:1245
@@ -1244,1 +1244,3 @@
 
+  void writeFragmentPadding(const MCFragment &F, uint64_t FSize,
+                            MCObjectWriter *OW) const;
----------------
eliben wrote:
> Here and elsewhere - please document all the new functions/methods you add - explaining their arguments, return values, etc.
Done for all the new functions/methods.

================
Comment at: include/llvm/MC/MCObjectStreamer.h:86
@@ +85,3 @@
+  // they get assigned, either to F if possible or to a new data fragment.
+  void flushPendingLabels(MCFragment *F, uint64_t FOffset = 0);
+
----------------
eliben wrote:
> What is FOffset for?
I have update the comment to address that.

================
Comment at: lib/MC/MCAssembler.cpp:633
@@ -630,3 +632,3 @@
   //   Prev  |##########|       F        |
   // -------------------------------------
   //                    ^
----------------
eliben wrote:
> Does this comment need updating?
I have update the comment to mention the optimization used when relax all flag is used.

================
Comment at: tools/pnacl-llc/pnacl-llc.cpp:706
@@ -705,3 +705,3 @@
   const Target *TheTarget = TargetRegistry::lookupTarget(MArch, TheTriple,
                                                          Error);
   if (!TheTarget) {
----------------
eliben wrote:
> This file doesn't see to belong here
Right, this is a leftover testing artifact; removed.

http://reviews.llvm.org/D8072

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list