[PATCH] [MC] Proper handle labels when -mc-relax-all flag is used

Petr Hosek phosek at chromium.org
Wed Jun 10 23:33:29 PDT 2015


================
Comment at: lib/MC/MCELFStreamer.cpp:77
@@ +76,3 @@
+      size_t Offset = DF->getContents().size();
+      for (const MCSymbol &Symbol : Assembler.symbols()) {
+        MCSymbolData &SD = Symbol.getData();
----------------
mseaborn wrote:
> mseaborn wrote:
> > This logic looks OK.  However, doesn't this take O(n) time in the number of symbols that have been created so far?  I'm concerned that this could lead to compilation taking O(n^2) time in the size of the module.  (Quadratic time per function would be bad, but quadratic time per module would be even worse. :-( )
> Here's a suggestion to avoid the O(n) time problem.
> 
> Background:
> 
>  * When PendingLabels was introduced by Derek (in http://llvm.org/viewvc/llvm-project?view=revision&revision=220439), it had the following invariant: If the current fragment is an MCDataFragment, PendingLabels should be empty.
>  * Functions such as EmitBytes() assumed they can add to the current data fragment, if there is one, without calling flushPendingLabels() first.
>  * Your change (r234714) created cases where the invariant didn't apply, which broke EmitBytes().  (If there had been an assertion to check the invariant, we would have noticed the problem more readily.)
> 
> How about we fix this assumption by EmitBytes() etc., by making sure they trigger a call to flushPendingLabels()?  Maybe we could have getOrCreateDataFragment() call flushPendingLabels().  We would then allow PendingLabels to be non-empty when the current fragment is an MCDataFragment.
> 
> That way, we don't have to adjust any labels after they've already been assigned to an offset within a fragment.
I agree, that's the solution I'm currently considering i.e. calling `flushPendingLabels()` from `getOrCreateDataFragment()`. I still need to run the change through bots to make sure that everything it doesn't break any other invariants.

http://reviews.llvm.org/D10325

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






More information about the llvm-commits mailing list