poorna2152 / nballerina

WebAssembly Backend for the nBallerina compiler
https://ballerina.io/
Apache License 2.0
4 stars 0 forks source link

Refactor code #48

Closed poorna2152 closed 2 years ago

poorna2152 commented 2 years ago

The code for checking overflows is currently added inline. For example checking overflows in addition,

    (if 
      (i32.and 
        (i64.gt_s 
          (local.get $0) 
          (i64.const 0)) 
        (i64.gt_s 
          (local.get $1) 
          (i64.const 0))) 
      (if 
        (i64.gt_s 
          (local.get $0) 
          (i64.sub 
            (i64.const 9223372036854775807) 
            (local.get $1))) 
        (throw $overflow)) 
      (if 
        (i32.and 
          (i64.lt_s 
            (local.get $0) 
            (i64.const 0)) 
          (i64.lt_s 
            (local.get $1) 
            (i64.const 0))) 
        (if 
          (i64.lt_s 
            (local.get $0) 
            (i64.sub 
              (i64.const -9223372036854775808) 
              (local.get $1))) 
          (throw $overflow))))

This is currently generated using functions in the print.wasmmodule with a code which looks ugly https://github.com/poorna2152/nballerina/blob/wback_subset07/compiler/modules/wback/number.bal#L80-L140.

Is it okay to create a runtime wat function rather than doing it inline. When implementing subset 02 we discussed to have it inline. I am guessing that is as an optimization. But if I create a function then the wasm-opt tool is going to inline it anyway. That would be more work to the optimizer but the code would be clean. Shall I make it as a function

manuranga commented 2 years ago

If it's getting inlined later, function sounds like a good idea to me.