i#2974 trace support for AArch64, part 2: added trace implementation #5045

Vincent-lau · 2021-08-11T19:48:42Z

This patch incorporated changes from patch #2442, and fixed some corner cases
where the assumption was incorrect and caused the program to crash.

Trace support is not yet enabled by default, but can be enabled with "-enable_traces".

A large part of this work was based on work done by Kevin Zhou. Please see branch
i1569-trace and patch #2442 for the original version.

Issues: #1569, #2974

…part of support This patch incorporated changes from patch #2442, and fixed some corner cases where the assumption was incorrect and caused the program to crash. Trace support is not yet enabled by default, but can be enabled with "-enable_traces". A large part of this work was based on work done by Kevin Zhou. Please see branch i1569-trace and patch #2442 for the original version. Issues: #1569, #2974

Vincent-lau · 2021-08-12T09:05:40Z

The results for the failure on Jenkins are

debug-internal-64: 242 tests passed, **** 5 tests failed, of which 1 were flaky, but ignoring 4 for i#2417: ****
	(ignore: i#2417) 	code_api|linux.fib-conflict 
	(ignore: i#2417) 	code_api|linux.fib-conflict-early 
	(ignore: i#2417) 	code_api|tool.drcachesim.phys 
	(ignore: i#2417) 	code_api|api.rseq 
	code_api|api.static_sideline_FLAKY 

====> FAILURE in release-external-64 <====
release-external-64: 48 tests passed, **** 3 tests failed, but ignoring 2 for i#2417: ****
	(ignore: i#2417) 	code_api|tool.drcachesim.phys 
	(ignore: i#2417) 	code_api|tool.drcacheoff.rseq 
	code_api|tool.drcachesim.coherence

which I believe are flaky tests by #2417

AssadHashmi · 2021-08-12T14:57:17Z

which I believe are flaky tests by #2417

Yes AIUI. We need to add that to the FLAKY_ list.

AssadHashmi · 2021-08-12T14:57:27Z

run arm tests

core/arch/interp.c

Removed all the commented code and an unused parameter. Issues: #1569, #2974

abhinav92003 · 2021-08-16T14:14:44Z

Thanks for making the changes. Will take a look today

Vincent-lau · 2021-08-16T16:07:20Z

Thanks for making the changes. Will take a look today

I just noticed dcontext in check_patch() is also redundant, I will change it along with any other changes that you might propose.

abhinav92003

Mid review comments.

core/arch/emit_utils_shared.c

core/arch/interp.c

core/arch/aarch64/emit_utils.c

Some clean up according to the review. Also changing the implementation of check_patched_ibl to merge two checks into one function. Issues: #1569, #2974

core/arch/interp.c

abhinav92003 · 2021-08-18T05:02:56Z

Thanks for making the previous changes. In most of my recent comments, I'm just trying to understand some parts of trace building that I'm not familiar with yet, and maybe help improve inline documentation along the way.

Vincent-lau · 2021-08-19T19:14:41Z

Thanks for all the comments. as I said, the original author of this code was not me, and I have to guess the intent of the author (just as you do). But I have replied to your review with my thoughts.

Replaced some of the if-conditions with assertions and improved code comments. Issues: #1569, #2974

Vincent-lau · 2021-08-23T15:19:48Z

One other thing, I am thinking about making the trace support for AArch64 on by default (after a few more iterations), I am wondering roughly how much work would be needed for that? I would imagine that the bare minimum is to pass all current tests with trace on by default and we might need to close the issue #5057 as well?

core/arch/interp.c

abhinav92003 · 2021-08-24T02:37:07Z

One other thing, I am thinking about making the trace support for AArch64 on by default (after a few more iterations), I am wondering roughly how much work would be needed for that? I would imagine that the bare minimum is to pass all current tests with trace on by default and we might need to close the issue #5057 as well?

Atleast some targeted test coverage would be good, yes. In addition to this, we'd need to run some large internal apps/benchmarks under DR with traces enabled. In the past, we've seen that some issues show up only on large apps, not on unit tests or SPEC.

Changed the condition of checking for mangled stolen register to include the ldr instruction. Added comment on clarifying the need to iterate over the entire trace to find all indirect exits. Issues: #1569, #2974

Vincent-lau · 2021-08-24T15:49:11Z

Atleast some targeted test coverage would be good, yes.

I am thinking about adding tests to cover some of the cases introduced in this PR, but not quite sure how to get started. My initial thought is to write a basic block such as

br  %x3
b  ibl_lookup_routine

And it would be mangled into

str %x2 -> IBL_TARGET_SLOT
mov %x3 -> %x2
b ibl_lookup_routine

And then we expect it to be fixed up as

str  %x0 -> TLS_REG0_SLOT
mov #trace_next_target -> %x0
eor %x0 %x3 -> %x0
cbnz %x0 trace_exit
ldr TLS_REG0_SLOT -> %x0

So that we are covering one of the cases introduced by mangle_indirect_branch_in_trace

I problem I have, though, with this idea is that I am not sure if it is actually possible for the compiler to generate such assembly instructions in a bb. On the other hand, if I write tests in C, then I can be sure that it is legitimate code generated by the compiler but can't be sure that it will cover the cases introduced by this PR.

Vincent-lau · 2021-08-24T15:50:32Z

we'd need to run some large internal apps/benchmarks under DR with traces enabled

Can you suggest some applications to run?

abhinav92003 · 2021-08-25T04:31:22Z

I am thinking about adding tests to cover some of the cases introduced in this PR, but not quite sure how to get started. My initial thought is to write a basic block such as
br  %x3
b  ibl_lookup_routine

Note that the b ibl_lookup_routine will not be present in the test app itself; it will be added by DR.

I problem I have, though, with this idea is that I am not sure if it is actually possible for the compiler to generate such assembly instructions in a bb. On the other hand, if I write tests in C, then I can be sure that it is legitimate code generated by the compiler but can't be sure that it will cover the cases introduced by this PR.

You can add your test basic blocks in assembly, similar to drreg tests that are also written in assembly to verify behaviour for some hand-crafted basic blocks. See drreg-test.c (the test app for drreg) and drreg-test.dll.c (the test client for drreg).

I guess you can use dr_register_trace_event and dr_register_end_trace_event to register callbacks for trace events, and in those callbacks you can verify the instructions in the trace. I have never looked at a trace closely myself, so please verify how the traces look on these events.

Vincent-lau · 2021-08-25T13:28:57Z

I guess you can use dr_register_trace_event and dr_register_end_trace_event to register callbacks for trace events, and in those callbacks you can verify the instructions in the trace

Thanks for the suggestions. I had a look at these register_events methods and they seem to me to be client-side, and DR does not seem to have methods that expose the fragment to the client (which is the transparency of DR I suppose?). As most of the modifications made by the code is to transform the bb so that it can be emitted into the code cache correctly, these clients do not help much. I guess my question is if there are any APIs on the client side that can inspect the structure of a fragment as it gets emitted or if there is any way for a client to look at instructions modified by DR? If not, then we probably need to think of other ways to do the test.

I might suggest writing some tests similar to what null_instrument.c did, which is to test if a trace is correctly created if some bbs are executed frequently enough. For example, we can create some control flow and execute one condition for some time and the other for some time, and see if they have traces created respectively. This test can be useful for x86 as well, so I might start working on that.

abhinav92003

I had a look at these register_events methods and they seem to me to be client-side, and DR does not seem to have methods that expose the fragment to the client

With dr_register_trace_event you can register a callback that takes the trace as input. The documentation also states that
"trace is a pointer to the list of instructions that comprise the trace." and " The user is free to inspect and modify the non control-flow instructions in the trace before it executes, with certain restrictions". Are you saying that the given trace is not the one after the trace creation code runs?

void dr_register_trace_event(dr_emit_flags_t (*func)(void *drcontext, void *tag,
                                                     instrlist_t *trace,
                                                     bool translating));

dynamorio/core/lib/dr_events.h

Line 392 in 271d357

* The user is free to inspect and modify the non-control-flow

core/arch/interp.c

core/arch/aarch64/emit_utils.c

abhinav92003 · 2021-08-26T22:28:18Z

@Vincent-lau I think it should be okay if we're not able to add tests in this PR itself. That can be done as follow-up in #5057.

@derekbruening We iterated on this implementation and I think the overall approach makes sense. Can you take a look too? To make sure that I'm not missing any details or if anything can be simplified. In addition, I've also tagged you in some comments where we could use your input.

core/arch/aarch64/emit_utils.c

core/arch/interp.c

derekbruening · 2021-08-27T05:12:42Z

core/arch/interp.c

+ * If we delete all code after "b target", then the "fall" path would
+ * be lost. Therefore we need to append the fall path at the end of
+ * the trace as a fake exit stub. Swapping them might be dangerous since
+ * a stub trace may be created on both paths.


If we switch to #5062 and append unmangled instrlists and only mangle at the very end, all this complexity would go away, right?

only mangle at the very end

So you mean that we build a trace from scratch and only mangle when we have all the bbs stitched together? I think this would indeed remove this complexity.

I might bring up one other problem related to this as well, in translate.c the function recreate_app_state_from_ilist searches the recreated ilist for the instruction that has the corresponding app pc of the current mc->pc. With the this new patch adding appending instructions on the miss path of the cbnz x0, trace_exit_label:

* For example, the code will be fixed to: * eor x0, x0, jump_target * cbnz x0, trace_exit_label * ... * b trace_head * trace_exit_label: * ldr x0, TLS_REG0_SLOT * str x2, TLS_REG2_SLOT * mov x2, jump_target * b ibl_routine */

The recreate_app_state_from_ilist is not able to find the instr if the pc happens to be in one of the instructions following trace_exit_label and since these instructions are not identified as a exit stub, the mc->pc wouldn't be pointing to the correct cti either, and as they don't belong to the application either, so won't appear in the recreated ilist, recreate_app_state_from_ilist is not able to find the correct instruction and reaches the ASSERT_NOT_REACHED on L1114.

So you mean that we build a trace from scratch and only mangle when we have all the bbs stitched together? I think this would indeed remove this complexity.

Yes. We already have the functionality in mangle_trace() to do all the bb stitching at the very end: we have to do that for clients that change the trace in the trace event (the event passes them the unmangled instrlist).

Are you interested in implementing #5062? It would be a nice cleanup and simplify this PR which would then come in afterward.

The recreate_app_state_from_ilist is not able to find the instr if the pc happens to be in one of the instructions following trace_exit_label

That's a blocking issue that has to be solved.

One proposal is to change the i.b. comparison to use x2 instead of x0 as the temp reg holding the expected target immediate. Then the only thing missing is copying from the actual target reg into x2, and maybe that could be done locally. It would use a taken forward branch on the match which maybe is not ideal but certainly simple:

str x2, TLS_REG2_SLOT mov x2, #trace_next_target eor x2, x2, jump_target cbz x2, match mov x2, jump_target b ibl_routine_regular_exit_stub match: ldr x2, TLS_REG2_SLOT

Then we eliminate this special-case trace_exit_label.

(If jump_target == x2 then a different scheme needs to be used -- might require 2 spills b/c we want to put the app value in the x2 slot and have a local scratch.)

Note that whatever mangling is added, translate.c needs to be updated to recognize it. Basic spills and restores are already recognized but anything else may need special support.

what would happen if target_cache is pointing to one of these mangled instructions

The uncompleted mangled app instruction will be re-executed, so all state changes have to be rolled back: restore spilled registers, etc.

Are you interested in implementing #5062?

How long do you estimate this would take? I am currently on the final week of an internship where I try to port the trace support for a64. Obviously I can continue contributing after the internship but I am just trying to get a sense of how long this is going to take.

I don't know for sure -- it might be quite straightforward:

go enable things permanently that are currently gated on monitor_data_t.pass_to_client like extend_unmangled_ilist's work

change internal_extend_trace to estimate the size from the unmangled list (maybe w/ some fudge factor for mangling: for new scheme only used to keep trace below max sizes I would assume)

delete monitor_data_t.trace_buf, extend_trace, etc.

try to delete decode_fragment: check other callers

But there might be some extra things to deal with once in the details.

Maybe better to finish this PR off as a deliverable -- but the missing translation support is an issue. Could revert the optimization and have sub-optiimal code that matches x86 with the spill-and-move left there and a simple exit stub and complete translation support, with a comment that we can improve the perf later.

The uncompleted mangled app instruction will be re-executed, so all state changes have to be rolled back: restore spilled registers, etc.

In that case is it OK to include these instructions in the recreated ilist (currently they are not)

* trace_exit_label: * ldr x0, TLS_REG0_SLOT * str x2, TLS_REG2_SLOT * mov x2, jump_target * b ibl_routine

And store the translation info in these instructions when we call fixup_indirect_trace_exit?

In that case is it OK to include these instructions in the recreated ilist (currently they are not)

Wait -- why are they not in the recreation of the trace? That is not good. It should go through the same mangling steps.

Because these instructions are added in fixup_indirect_trace_exit which is not called when recreate_fragment_ilist runs. I have added a call in recreate_fragment_ilist so that these instructions are included in the recreated instrlist as well.

Vincent-lau · 2021-08-27T14:27:22Z

Are you saying that the given trace is not the one after the trace creation code runs?

I will move the discussion to #5057

fixup_indirect_trace_exit Added translation information when fixing up the indirect trace exits and added call to fixup_indirect_trace_exit when recreating the fragment ilist Issues: #1569, #2974

derekbruening · 2021-08-30T20:24:25Z

Seeing unresolved comments: please resolve when addressed

core/arch/interp.c

derekbruening · 2021-08-30T20:37:11Z

core/arch/interp.c

+ * If we delete all code after "b target", then the "fall" path would
+ * be lost. Therefore we need to append the fall path at the end of
+ * the trace as a fake exit stub. Swapping them might be dangerous since
+ * a stub trace may be created on both paths.


In that case is it OK to include these instructions in the recreated ilist (currently they are not)

Wait -- why are they not in the recreation of the trace? That is not good. It should go through the same mangling steps.

core/arch/interp.c

derekbruening · 2021-08-30T20:42:16Z

core/arch/interp.c

+ * the trace as a fake exit stub. Swapping them might be dangerous since
+ * a stub trace may be created on both paths.
+ */
+


Please include TODO i#xxxx comments (with the real issue number) for the missing pieces: in particular, adding translation support for all the new mangling code. (Ideally it would be in this PR but for making progress here w/ a new feature in pieces ok to not have all at once so long as what's missing is clearly documented not just in the commit description but in the code too). I would put such comments both here and in translate.c.

I feel that there isn't any translation support needed for the new mangling code. The new mangling code includes:

* Original ibl lookup: * str tgt_reg, TLS_REG2_SLOT * mov tgt_reg, jump_target * b ibl_routine * Now we rewrite it into: * str x0, TLS_REG0_SLOT * mov x0, #trace_next_target * eor x0, x0, jump_target * cbnz x0, trace_exit (ibl routine) * ldr x0, TLS_REG0_SLOT */

and

* For example, the code will be fixed to: * eor x0, x0, jump_target * cbnz x0, trace_exit_label * ... * b trace_head * trace_exit_label: * ldr x0, TLS_REG0_SLOT * str x2, TLS_REG2_SLOT * mov x2, jump_target * b ibl_routine

which are just basic spilling of registers and perform some calculation after spilling, I suppose? What sort of support do you think would be needed for these new mangling instructions?

The issue I mentioned previously about recreate_fragment_ilist not being able to find an instruction was purely because the new mangled instructions added by fixup_indirect_trace_exit is not added when creating the fragment ilist.

Please run with -stress_recreate_state and verify there are no translation failures or warnings about unknown mangling sequences.

Looks like the "unsupported mangle instr" message is at DOLOG(2 -- I thought it was more visible esp with -stress_recreate_state. Please run with -loglevel 2 -stress_recreate_state and ensure there are no "unsupported mangle instr" messages in the log files.

I think there are already quite a lot of unsupported mangle instr even without trace enabled. I was not able to find any issues mentioning this so I opened a new one #5069. Feel free to point out if there are already issues that are talking about this problem.

This is surprising since I put in work earlier this year to enable -stress_recreate_state, fix a number of translation problems on aarch64, and enable a regression test: #4680. Taking a look.

To make progress let's merge this but please first add a comment in the code and in the commit message about these separate trace_exit_label sequences breaking translation and how either special support needs to be added or (the better choice IMHO: best to avoid internal control flow) they need to be removed and a different scheme used such as the one proposed at https://github.com/DynamoRIO/dynamorio/pull/5045/files/b9ada873ecc0b86128169aae0bd7ad648c0f921b..fc70e80bad8a66b7f417c27673fa5659e96ff1ef#r697566139

the better choice IMHO: best to avoid internal control flow

I agree with you that a linear control flow is preferable, will add a comment in the code for trace_exit_label

This is surprising since I put in work earlier this year to enable -stress_recreate_state, fix a number of translation problems on aarch64, and enable a regression test: #4680. Taking a look.

I am guessing it's because these instructions are just reported with a log message, which did not catch your attention while you were fixing them?

I recall looking at such log messages -- but I think those memories are from rseq translation work #4316 and the #4680 is as you said: focusing on asserts and fatal problems.

simpler approach Added a comment on the possibility to switch to a simpler approach of not decoding from code cache. And changed setting translation of instructions using INSTR_XL8 macro. Issues: #1569, #2974

bound Fixed a problem where the branch upper bound is calculated correctly. Issues: #1569, #2974

Fixed clang-format issue. Issues: #1569, #2974

core/arch/interp.c

derekbruening · 2021-08-31T17:42:53Z

core/arch/arch_exports.h

@@ -2196,6 +2196,11 @@ int
 append_trace_speculate_last_ibl(dcontext_t *dcontext, instrlist_t *trace,
                                app_pc speculate_next_tag, bool record_translation);

+#ifdef AARCH64
+int
+fixup_indirect_trace_exit(dcontext_t *dcontext, instrlist_t *trace);


See below: long-term this should only be inside mangle_trace() and so this would be removed when #5062 is implemented. Please add a XXX i#5062 comment about this.

Instead of 'here', maybe mention end_and_emit_trace.

derekbruening · 2021-08-31T23:01:31Z

core/arch/interp.c

+ * the trace as a fake exit stub. Swapping them might be dangerous since
+ * a stub trace may be created on both paths.
+ */
+


Please run with -stress_recreate_state and verify there are no translation failures or warnings about unknown mangling sequences.

wrong instr Issues: #1569, #2974

optimisation Issues: #1569, #2974

instr_is_trace_cmp for AArch64 Issues: #1569, #2974

mangled instr Issues: #1569, #2974

Fixed clang-format issue. Issues: #1569, #2974

derekbruening · 2021-09-01T16:20:14Z

core/arch/interp.c

+ * the trace as a fake exit stub. Swapping them might be dangerous since
+ * a stub trace may be created on both paths.
+ */
+


To make progress let's merge this but please first add a comment in the code and in the commit message about these separate trace_exit_label sequences breaking translation and how either special support needs to be added or (the better choice IMHO: best to avoid internal control flow) they need to be removed and a different scheme used such as the one proposed at https://github.com/DynamoRIO/dynamorio/pull/5045/files/b9ada873ecc0b86128169aae0bd7ad648c0f921b..fc70e80bad8a66b7f417c27673fa5659e96ff1ef#r697566139

core/arch/aarch64/emit_utils.c

linear control flow Currently fixup_indirect_trace_exit adds a special trace_exit_label that breaks the linear control flow which is assumed by many places in the code. We need to consider an alternative scheme to avoid this problem or add special support for the trace_exit_label. Also reverted changes on incorrect upper branch bound calculation. Issues: #1569, #2974

Vincent-lau · 2021-09-01T18:33:44Z

It seems that I need to add a lot in the final commit message 😄 , might as well include it here before I merge this into the master branch just in case I forgot something.

i#2974 trace support for AArch64, part 2: implementation of the main part of support 

This patch incorporated changes from patch #2442, and fixed some corner cases
where the assumption was incorrect and caused the program to crash.

Trace support is not yet enabled by default, but can be enabled with "-enable_traces".

This commit introduces internal control flow by adding a trace_exit_label in
fixup_indirect_trace_exit, which might break code that assumes linear control
flow. Either special support is needed for this trace_exit_label or alternative
schemes using linear control.

Co-authored-by: Kevin Zhou <[email protected]>

Issues: #1569, #2974

derekbruening

Thank you for the PR, and taking the time to go through what ended up being a long review process!

derekbruening · 2021-09-01T19:03:34Z

core/arch/aarch64/emit_utils.c

@@ -370,14 +370,14 @@ patch_branch(dr_isa_mode_t isa_mode, cache_pc branch_pc, cache_pc target_pc,
    ASSERT(ALIGNED(branch_pc, 4) && ALIGNED(target_pc, 4));


i#2974 trace support for AArch64, part 18: Adding comment on preferring
linear control flow

Note that the ", part N: " is a convention for merges to master: i.e., this PR being merged will be part 1; the next PR for trace building merged into master would be part 2 (not part 19 or sthg: the PR-internal ones don't count toward the master merges).

sure, that's why I put part 2 in my final commit message above. Have a look if you want and see if there is anything missing or not quite right.

Vincent-lau · 2021-09-01T19:07:03Z

Thank you for the PR, and taking the time to go through what ended up being a long review process!

No problem at all, really appreciated your (and other reviewer's) speedy reply!

abhinav92003

Thanks for the discussion and addressing all comments!

abhinav92003 · 2021-09-01T19:50:35Z

core/arch/arch_exports.h

@@ -2196,6 +2196,11 @@ int
 append_trace_speculate_last_ibl(dcontext_t *dcontext, instrlist_t *trace,
                                app_pc speculate_next_tag, bool record_translation);

+#ifdef AARCH64
+int
+fixup_indirect_trace_exit(dcontext_t *dcontext, instrlist_t *trace);


Instead of 'here', maybe mention end_and_emit_trace.

fixedup_indirect_trace_exit Issues: #1569, #2974

abhinav92003 reviewed Aug 12, 2021

View reviewed changes

core/arch/interp.c Outdated Show resolved Hide resolved

core/arch/interp.c Outdated Show resolved Hide resolved

core/arch/interp.c Outdated Show resolved Hide resolved

i#2974 trace support for AArch64, part 3: removed comments in code

8a14b7b

Removed all the commented code and an unused parameter. Issues: #1569, #2974

Vincent-lau requested a review from abhinav92003 August 13, 2021 12:57

abhinav92003 reviewed Aug 17, 2021

View reviewed changes

i#2974 trace support for AArch64, part 4: Code clean up

7b5b87d

Some clean up according to the review. Also changing the implementation of check_patched_ibl to merge two checks into one function. Issues: #1569, #2974

abhinav92003 reviewed Aug 18, 2021

View reviewed changes

i#2974 trace support for AArch64, part 5: Adding assertions

d0a5ff4

Replaced some of the if-conditions with assertions and improved code comments. Issues: #1569, #2974

Vincent-lau requested a review from abhinav92003 August 23, 2021 10:45

abhinav92003 requested changes Aug 24, 2021

View reviewed changes

core/arch/interp.c Show resolved Hide resolved

core/arch/interp.c Outdated Show resolved Hide resolved

i#2974 trace support for AArch64, part 6: Code clean up

fc4fdd4

Changed the condition of checking for mangled stolen register to include the ldr instruction. Added comment on clarifying the need to iterate over the entire trace to find all indirect exits. Issues: #1569, #2974

Vincent-lau requested a review from abhinav92003 August 26, 2021 13:29

abhinav92003 reviewed Aug 26, 2021

View reviewed changes

core/arch/interp.c Show resolved Hide resolved

core/arch/interp.c Outdated Show resolved Hide resolved

core/arch/interp.c Show resolved Hide resolved

core/arch/interp.c Show resolved Hide resolved

core/arch/aarch64/emit_utils.c Show resolved Hide resolved

abhinav92003 requested a review from derekbruening August 26, 2021 22:32

derekbruening reviewed Aug 27, 2021

View reviewed changes

i#2974 trace support for AArch64, part 7: Add translation info in

b9ada87

fixup_indirect_trace_exit Added translation information when fixing up the indirect trace exits and added call to fixup_indirect_trace_exit when recreating the fragment ilist Issues: #1569, #2974

Vincent-lau requested a review from derekbruening August 30, 2021 19:18

derekbruening reviewed Aug 30, 2021

View reviewed changes

Vincent-lau added 3 commits August 31, 2021 10:14

i#2974 trace support for AArch64, part 9: Added comments on switching to

e5b89cb

simpler approach Added a comment on the possibility to switch to a simpler approach of not decoding from code cache. And changed setting translation of instructions using INSTR_XL8 macro. Issues: #1569, #2974

i#2974 trace support for AArch64, part 11: Fixed incorrect branch upper

5b20ed0

bound Fixed a problem where the branch upper bound is calculated correctly. Issues: #1569, #2974

i#2974 trace support for AArch64, part 12: Fixed clang-format issue

d09d2c9

Fixed clang-format issue. Issues: #1569, #2974

Vincent-lau requested a review from derekbruening August 31, 2021 12:57

derekbruening reviewed Aug 31, 2021

View reviewed changes

Vincent-lau added 2 commits September 1, 2021 09:49

i#2974 trace support for AArch64, part 13: Fixed a bug of appending the

88c535d

wrong instr Issues: #1569, #2974

i#2974 trace support for AArch64, part 14: Adding comments on

ed826fd

optimisation Issues: #1569, #2974

Vincent-lau mentioned this pull request Sep 1, 2021

finish support for translating mangled instructions on AArch64 #5069

Open

Vincent-lau added 3 commits September 1, 2021 10:55

i#2974 trace support for AArch64, part 15: Implementing

6c7cf64

instr_is_trace_cmp for AArch64 Issues: #1569, #2974

i#2974 trace support for AArch64, part 16: Adding comment on unsupported

a2047ba

mangled instr Issues: #1569, #2974

i#2974 trace support for AArch64, part 17: Fixed clang-format issue

fc70e80

Fixed clang-format issue. Issues: #1569, #2974

Vincent-lau requested a review from derekbruening September 1, 2021 12:56

derekbruening approved these changes Sep 1, 2021

View reviewed changes

derekbruening requested changes Sep 1, 2021

View reviewed changes

core/arch/aarch64/emit_utils.c Outdated Show resolved Hide resolved

Vincent-lau requested review from abhinav92003 and derekbruening September 1, 2021 18:34

derekbruening approved these changes Sep 1, 2021

View reviewed changes

abhinav92003 approved these changes Sep 1, 2021

View reviewed changes

Vincent-lau added 2 commits September 1, 2021 21:12

i#2974 trace support for AArch64, part 19: Improved comments about

4dffc58

fixedup_indirect_trace_exit Issues: #1569, #2974

Merge branch 'master' into i2974-trace

3012697

Vincent-lau merged commit 8469b44 into master Sep 1, 2021

Vincent-lau deleted the i2974-trace branch September 1, 2021 21:59

egrimley mentioned this pull request Nov 29, 2023

i#1569 AArch64: Implement trace optimisation. #2442

Closed

		@@ -370,14 +370,14 @@ patch_branch(dr_isa_mode_t isa_mode, cache_pc branch_pc, cache_pc target_pc,
		ASSERT(ALIGNED(branch_pc, 4) && ALIGNED(target_pc, 4));

i#2974 trace support for AArch64, part 2: added trace implementation #5045

i#2974 trace support for AArch64, part 2: added trace implementation #5045

Conversation

Vincent-lau commented Aug 11, 2021

Vincent-lau commented Aug 12, 2021 • edited Loading

AssadHashmi commented Aug 12, 2021

AssadHashmi commented Aug 12, 2021

abhinav92003 commented Aug 16, 2021

Vincent-lau commented Aug 16, 2021

abhinav92003 left a comment

Choose a reason for hiding this comment

abhinav92003 commented Aug 18, 2021

Vincent-lau commented Aug 19, 2021 • edited Loading

Vincent-lau commented Aug 23, 2021 • edited Loading

abhinav92003 commented Aug 24, 2021

Vincent-lau commented Aug 24, 2021 • edited Loading

Vincent-lau commented Aug 24, 2021

abhinav92003 commented Aug 25, 2021

Vincent-lau commented Aug 25, 2021 • edited Loading

abhinav92003 left a comment

Choose a reason for hiding this comment

abhinav92003 commented Aug 26, 2021

Choose a reason for hiding this comment

Vincent-lau Aug 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derekbruening Aug 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vincent-lau Aug 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vincent-lau commented Aug 27, 2021

derekbruening commented Aug 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vincent-lau Aug 31, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vincent-lau Sep 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vincent-lau commented Sep 1, 2021 • edited Loading

derekbruening left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vincent-lau Sep 1, 2021 • edited Loading

Choose a reason for hiding this comment

Vincent-lau commented Sep 1, 2021

abhinav92003 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vincent-lau commented Aug 12, 2021 •

edited

Loading

Vincent-lau commented Aug 19, 2021 •

edited

Loading

Vincent-lau commented Aug 23, 2021 •

edited

Loading

Vincent-lau commented Aug 24, 2021 •

edited

Loading

Vincent-lau commented Aug 25, 2021 •

edited

Loading

Vincent-lau Aug 27, 2021 •

edited

Loading

derekbruening Aug 27, 2021 •

edited

Loading

Vincent-lau Aug 28, 2021 •

edited

Loading

Vincent-lau Aug 31, 2021 •

edited

Loading

Vincent-lau Sep 1, 2021 •

edited

Loading

Vincent-lau commented Sep 1, 2021 •

edited

Loading

Vincent-lau Sep 1, 2021 •

edited

Loading