[ +Slava ] Answers inline ...
On Tue, Jun 16, 2015 at 6:06 PM, Mircea Trofin <[email protected]> wrote: > I was thinking more about this. We currently appear to have, one way or > the other, an inconsistency when it comes to offsets: ll_prof outputs > offsets in hex, the d8 pretty printers output in dec. > > At a high level, what do we believe is the right thing to do here? Do we > want them in decimal everywhere (case in which I'll modify ll_prof or > disasm.py) or do we want them in hex everywhere? > Whether we want this or not, I cannot answer. I don't have a stake in this discussion. The thing I care about is internal consistency within V8. While the latter may be a more unpleasant path, I feel it is the better > answer: if we start using other tools for analysis - just like ll_prof > today reuses objdump - I think hex will prove to be the common format for > offsets, rather than dec. > > If we agree on that, do we have a comprehensive list of places we print > offsets? That would make the change straightforward. > Nope, such a list does not exist. But everything printed by Code::CodePrint is a good starting point. As I already mentioned partially in the revert reason, that includes in particular: - safepoint table - back edge table - handler table - deoptimization input data - deoptimization output data If no such list exists, I can see 2 options for making progress with > calculated risk: > > 1) introduce a flag, print hex by default. If someone discovers we missed > a spot, they may disable the flag in their runs (to unblock their work), > and either fix or file a bug. Eventually we remove the flag altogether. > > 2) no flag, cold turkey option. We do best effort upfront, and treat > subsequently discovered incorrectly formatted offsets as bugs, which we > then fix. > > I prefer option 2, because option 1 will likely complicate the code for > the interim until we're satisfied the change is complete. Option 2 seems > like would get us where we want fastest. > > Thoughts? > As I said before in my previous mail: "I am fine with the switch, if it's done consistently in one go". So that is option 2, no flag, pretty please. Also you might want to give Vyacheslav Egorov (aka. Slava) a heads up, because his tool called "IRHydra" might rely on parsing that output, not sure though. I CCed Slava. [ TL;DR for Slava: The context here is switching PC-offsets printed by --print-code and friends from decimal to hexadecimal. ] On Tue, Jun 16, 2015 at 6:36 AM Mircea Trofin <[email protected]> wrote: > >> >> >> The goal was to make correlating with "perf" output easy, so offsets are >> sufficient in that case. Would a flag to enable only offsets in hex address >> your concern? Something like --decomp-offsets-hex. >> >> If we feel we wanted a master switch instead (all numbers to hex), we >> could call the flag something like "--use-hex", and gradually (over many >> CLs) change other traces. But I don't know if we have scenarios asking for >> that just yet. >> >> >> Thoughts? >> >> Thanks! >> On Tue, Jun 16, 2015 at 03:36 <[email protected]> wrote: >> >>> A revert of this CL (patchset #2 id:20001) has been created in >>> https://codereview.chromium.org/1188093002/ by [email protected]. >>> >>> The reason for reverting is: Code printout has become unreadable. Offsets >>> are >>> printed in decimal numbers everywhere else. This is inconsistent with the >>> rest >>> of the code-base. Some examples are tables for deoptimization data, >>> safepoints >>> and exception handlers. I would be fine with this change if _all_ tracing >>> would >>> be adapted. But there are _many_ places to touch.. >>> >>> https://codereview.chromium.org/1177123002/ >>> >> -- -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
