[PATCH] D70062: MCObjectStreamer: assign MCSymbols in the dummy fragment to offset 0.

Peter Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 11 06:23:46 PST 2019


peter.smith added a comment.

This looks reasonable to me. The one concern I had was that Checking over the case where getRelaxAll() is true, it looks like each instruction is created in its own fragment, then merged with the previous, triggering a call to flushPendingLabels which will set the offset correctly before the temporary offset of 0 could be misused. Might be worth a test case for the combination of .if and --mc-relax-all.



================
Comment at: llvm/lib/MC/MCObjectStreamer.cpp:261
   else
     PendingLabels.push_back(Symbol);
 }
----------------
For consistency is it worth setting the offset to 0 here as well? From what I can tell this is only used for outputting Arm/Thumb mapping symbols and these really shouldn't be used for any kind of symbolic lookup, but it may be possible other uses for this function will be found in the future.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70062/new/

https://reviews.llvm.org/D70062





More information about the llvm-commits mailing list