Perform naiive branch relaxing (and other fixes to the BIR passes) #210
Conversation
Also pretty-print it with the rest of the WP params
Any explicit exits (blocks that short-circuit the flow of the patch away from the original flow of the program) are assumed to not need the higher vars
Where `n` is a power of two. It's common to see this with array accesses.
1. We need branch relaxing if the patch needs to perform a conditional branch to some far away region of code. In this case, the assembler will fail because the branch is too far away. We break it apart into a conditional branch to a local block, followed by an unconditional branch to the desired location if the condition is met. 2. The higher vars initialization may clobber the flags if it occurs at the same time as a `cmp x, y; bcc loc`. The scheduler is unaware that on Thumb, many instructions will implicitly set the flags.
… bmourad/branch-relaxing
| @@ -57,4 +57,4 @@ end | |||
| (** [run patch] creates the BIR from [patch], then applies a series of | |||
| transformations to it. The resulting code is then ready to be handed | |||
| off to the instruction selector. *) | |||
| val run : Data.Patch.t -> t KB.t | |||
| val run : Data.Patch.t -> patch_spaces:Data.Patch_space_set.t -> t KB.t | |||
There was a problem hiding this comment.
It is odd to me you need the available patches sites at this point in the pipeline. I suppose you are guessing how big jumps will be based on this? This just doesn't feel right.
There was a problem hiding this comment.
I agree, though there are limitations to doing it later down the pipeline. If we did this at the level of the patcher, where the code has already been generated, then we would need to intercept the error from the assembler and then manually adjust the generated assembly code (or the VIBES IR). Doing it at the BIR level is easier.
| @@ -184,6 +184,7 @@ let wp_params_to_string (wp_params : Wp_params.t) : string = | |||
| "user-fun-specs-orig: %s" (triple_list wp_params.user_func_specs_orig); | |||
| Printf.sprintf | |||
| "user-fun-specs-mod: %s" (triple_list wp_params.user_func_specs_mod); | |||
| Printf.sprintf "init-mem: %b" wp_params.init_mem; | |||
There was a problem hiding this comment.
Does this PR enable the "init-mem" feature?
There was a problem hiding this comment.
I was forgetting to pretty-print it with the rest of the config. I also changed it so that it is a JSON boolean instead of a string.
|
Seems like good stuff. Great work! I am worried that it feels like the patch_spaces being in bir_passes is ensnarling the codebase in itself, but I don't see an easy way around it. Everything is dependent on everything if you want it to be really good unfortunately. Is point 3 above correct? I think this is maybe because of a weakness of the schema of the config file. Each possible exit point should have it's own notion in general of where hvars need to be put. In other words we need a write-to relation: (addr, hvar, lowloc) |
|
Yes, it seems that this would be an avenue for future work. In general when we have a patch like: We could just assume that once we go to Meanwhile, the fallthrough to It would be nice to have some granularity per-exit about what kind of data dependency is needed. |
|
Anyway seems good to me. Rebase and merge when you want. You've got conflicts right now |
There's a number of changes here:
Bir_passes.Shape.split_on_conditional)goto L_0x1234, then this is not an implicit exit and we can assume that the higher vars aren't needed "at exit" in any particular register.lsl rt, [rn, rm, lsl #n]pattern to the current selector (results in much smaller code!)