Hi Luke, On Mon, Sep 14, 2020 at 11:21:58AM -0700, Luke Nelson wrote: > On Mon, Sep 14, 2020 at 11:08 AM Xi Wang <xi.wang@xxxxxxxxx> wrote: > > I don't think there's some consistent semantics of "offsets" across > > the JITs of different architectures (maybe it's good to clean that > > up). RV64 and RV32 JITs are doing something similar to arm64 with > > respect to offsets. CCing Björn and Luke. > > As I understand it, there are two strategies JITs use to keep track of > the ctx->offset table. > > Some JITs (RV32, RV64, arm32, arm64 currently, x86-32) track the end > of each instruction (e.g., ctx->offset[i] marks the beginning of > instruction i + 1). > This requires care to handle jumps to the first instruction to avoid > using ctx->offset[-1]. The RV32 and RV64 JITs have special handling > for this case, > while the arm32, arm64, and x86-32 JITs appear not to. The arm32 and > x32 probably need to be fixed for the same reason arm64 does. > > The other strategy is for ctx->offset[i] to track the beginning of > instruction i. The x86-64 JIT currently works this way. > This can be easier to use (no need to special case -1) but looks to be > trickier to construct. This patch changes the arm64 JIT to work this > way. > > I don't think either strategy is inherently better, both can be > "correct" as long as the JIT uses ctx->offset in the right way. > This might be a good opportunity to change the JITs to be consistent > about this (especially if the arm32, arm64, and x32 JITs all need to > be fixed anyways). > Having all JITs agree on the meaning of ctx->offset could help future > readers debug / understand the code, and could help to someday verify > the > ctx->offset construction. > > Any thoughts? The common strategy does make a lot of sense and yes, both patches will works assuming the ctx->offset ends up being what the JIT engine expects it to be. As I mentioned earlier we did consider both, but ended up using the later, since as you said, removes the need for handling the special (-1) case. Cheers /Ilias > > - Luke
