Lua Perf/AST Improvements - #41
Conversation
- Removes AddI, SubtractI, MultiplyI - Adds explicit reductions for all of these (and division too). We now don't have any non-native arithmetic operation in buddy allocator except for u64.mod (which is necessary)
- Adds a noteq binary operator and adds a specialized match case in Not to find and replace Not == calls with the new NotEq node
- Removes the need to call the bw_compl function (last non-native number operation in hyp allocator) - Also cleans up unused parameter for negate and fixes up spacing for these 2 unary ops
| if true then ( | ||
| let mult_expr = LuaS.Binary (Multiply (offset_expr, sizeof_expr)) in | ||
| LuaS.(Binary (Add (ptr_expr, mult_expr)))) | ||
| else |
There was a problem hiding this comment.
Are you SURE array shifts cannot overflow?
There was a problem hiding this comment.
fairplay, i'll make sure to reduce it
EDIT: I've kept the reduction with a u64 (since the underlying size type, size_t, is for all intents and purposes an unsigned 64 bit number). Hope we never actually get to the top most bit since there's no real way to make that addition work here :'). Maybe in the future, we can use the reduction node to catch/trigger failure states if we're exceeding bounds (at least for literals). Although we'd only want that in certain cases since comparsions etc are still ok. Ehh
| let array_shift_expr = | ||
| if true then | ||
| LuaS.(Binary (AddI (ptr_expr, Binary (MultiplyI (offset_expr, sizeof_expr))))) | ||
| if true then ( |
There was a problem hiding this comment.
this is from a while ago when I added the optimisation to inline array and member shifts. The false case has the old case (in case we want to retain more readability). Ideally, I'd have some sort of flags for various optimisations but the optimisations have kind of happened over time organically so haven't had time to build a proper way to maintain them.
| let prec' = precedence expr in | ||
| let pp = | ||
| match args with | ||
| | Not (Binary (Eq (a, b, true))) -> pp_expr ~prec (Binary (NotEq (a, b))) |
There was a problem hiding this comment.
Is there no inequality coming in from above? Why do it this late?
There was a problem hiding this comment.
The existing translation infrastructure (across both C Fulminate or Lua Fulminate) never explicitly makes a NotEquals. It builds Equals, and then surrounds it with a Not (see cn_to_ail_unop, or in buddy allocator's c fulm case, see cn_bool_not(cn_x_equality)). Changing this would require doing a bigger overhaul across all translation areas to parse the internal expression and then unroll it to generate the proper AST. In some ways, this is a similar problem to our number reduction. But given that this is one isolated case, I feel we can retain this in the printer.
2 participants
This PR:
ultto only u64 typesNotEqbinary operator type and replaces all calls to not == for primitive types with ~=bw_complwith native xorWith these changes, we have (mostly) cleaned up our inconsistent dealing of reductions in the AST construction vs in the pretty printer, and added some other niceties. We now have no calls to number library functions in the instrumented outputs of buddy or hyp allocator (save for u64.mod, which we need since Lua does mods differently from C).
NOTE: While cleaner, this did not really result in any noticeable perf improvements. My hunches are: