Implement negation of conditional expressions during simplification.

[maximumspatium]

Hmm, weird that I never faced a need for that during expression simplification. Otherwise, the code is there (COND.neg()), used e.g. for if structuring. Now with the testcase, I'll look into implementing it. (Or maybe if you're serious about playing with SABl, you'll be interested to look into it yourself, I'd say it should be doable in terms of xform_expr_infer.py).

Hey, it's my first attempt to implement condition negation during expression simplification as you suggested. Morevover, I added a shortcut that immediately returns None in the case a non-expression argument is passed. That prevents unneeded looping and pattern matching.

Let me know if the patch is okay or need any improvement..

[maximumspatium]

BTW, the test suite doesn't pass. It fails on tests/decomp/40000454-_xtos_set_exception_handler/40000454-_xtos_set_exception_handler.lst and is unrelated to my changes. The tests need to be fixed first.

[pfalcon]

What's the failure? Everything passes for me.

[maximumspatium]

What's the failure? Everything passes for me.

=== test_decomp === tests/decomp/40000454-_xtos_set_exception_handler/40000454-_xtos_set_exception_handler.lst --- tests/decomp/40000454-_xtos_set_exception_handler/40000454-_xtos_set_exception_handler.lst.exp 2017-12-28 20:50:56.397440186 +0100 +++ tests/decomp/40000454-_xtos_set_exception_handler/40000454-_xtos_set_exception_handler.lst.out 2017-12-28 21:46:43.119394677 +0100 @@ -16,7 +16,7 @@ $a3 = _xtos_p_none; // {'uses': ['40000474', '4000047a']} } // $a5 = _xtos_exc_handlertable + $a2_0 * 4; // {'uses': []} (dead);

• $a2 = (u32)(_xtos_c_handler_table + $a2_0 * 4); // {'uses': []}
• // $a2 = (u32)(_xtos_c_handler_table + $a2_0 4); // {'uses': []} (dead); // $a7 = $a3 + -_xtos_p_none; // {'uses': ['40000477']} (dead); if ($a3 + -_xtos_p_none != 0) { $a4 = _xtos_c_wrapper_handler; // {'uses': ['4000047c']} @@ -25,9 +25,9 @@ (u32)(_xtos_exc_handlertable + $a2_0 4) = $a4; // {'uses': []} // $a10 = (u32)(_xtos_c_handler_table + $a2_0 4) + -_xtos_p_none; // {'uses': ['40000481']} (dead); if ((u32)(_xtos_c_handler_table + $a2_0 4) + -_xtos_p_none == 0) {
• $a2 = 0x0; // {'uses': []}
• // $a2 = 0x0; // {'uses': []} (dead); } } else {
• $a2 = 0x0; // {'uses': []}
• // $a2 = 0x0; // {'uses': []} (dead); } } TESTS FAILED!

  
  Tested on Ubuntu 16.04 LTS with bash v4.3.48. The test suite doesn't ever run on macOS due to bash script incompatibilities.

[pfalcon]

Well, time to setup TravisCI...

Should be fixed in master.

[maximumspatium]

Should be fixed in master.

Yep. I've just fixed the test case affected by my changes.

[pfalcon]

Back to the patch. Thanks for taking a stab at it! But it's cheating ;-). I don't know if you noticed or not, but xform_expr_infer.py implements poorman's Prolog inference engine, i.e. tries to do stuff declaratively. That's how it's different from xform_expr.py, which implements imperative passes. So, your patch wouldn't belong to xform_expr_infer.py, but to xform_expr.py. But when I said it would go to xform_expr_infer.py, I meant that it should be tried to be done "declaratively". The "consequent" part in your code is good, no need to re-do work which COND.neg() does (but I'd add a TODO comment that creating COND from EXPR just to call .neg() on it sucks). But at least pattern matching should be done declaratively.

I've pushed https://github.com/pfalcon/ScratchABlock/commit/b926b985a706efe9be2b34c8beba66fdb8807e60 as an example of dealing with relational operations.

Please also follow commit message format of the existing codebase. (But commits for test updates should be separate from code commits, yeah).

Also, there should be unit-level tests. I have a lot of experimental code lying around, and I penalize myself (and make world better) by not pushing it to master unless there're tests. I give up regularly on that rule (tests are boring!), and even by looking at the stuff for these 1-2 weeks, you can see that with the fact that there's some testsuite, it's maybe 20% of what should be there.

You can add tests to expr-simplify1.lst, unless it warrants to create a new file.

[pfalcon]

You can add tests to expr-simplify1.lst, unless it warrants to create a new file.

Now should go in expr-simplify-cmp.lst: 2a3c7558b3b42b8971d80604a7a26513480855b8

[maximumspatium]

Thank you for your review. I'll do the proposed changes tonight...

[maximumspatium]

Here we go: 129ecee

Everything is now done "declaratively". I reuse the COND.NEG dictionary from core.py in order to achieve operation negation (is that still cheating? :)

("==", "!=", "<", "<=", ">=", ">") is being used in two patterns. Should I factor it out?

[pfalcon]

Looks good, thanks. I hope a test is also coming ;-).

("==", "!=", "<", "<=", ">=", ">") is being used in two patterns. Should I factor it out?

Let's do it later I guess.

[maximumspatium]

I hope a test is also coming

I've tried to extend expr-simplify-cmp.lst with the following line:

30 $a1 = !($a1 == $a2)

The test suite now fails with the following error:

Error while processing file: tests/expr-simplify-cmp.lst
Traceback (most recent call last):
  File "./apply_xform.py", line 216, in <module>
    changed = one_iter(input, output, iter_no)
  File "./apply_xform.py", line 181, in one_iter
    handle_file(args)
  File "./apply_xform.py", line 58, in handle_file
    raise e
  File "./apply_xform.py", line 55, in handle_file
    handle_file_unprotected(args)
  File "./apply_xform.py", line 76, in handle_file_unprotected
    dump_bblocks(cfg, f, no_graph_header=args.no_graph_header)
  File "/home/maxpol/Dokumente/ScratchABlock/core.py", line 845, in dump_bblocks
    p.print()
  File "/home/maxpol/Dokumente/ScratchABlock/core.py", line 833, in print
    self.bblock.dump(self.stream, 0, self.print_inst)
  File "/home/maxpol/Dokumente/ScratchABlock/core.py", line 69, in dump
    out = printer(s)
  File "/home/maxpol/Dokumente/ScratchABlock/core.py", line 816, in print_inst
    return self.inst_printer(inst)
  File "/home/maxpol/Dokumente/ScratchABlock/core.py", line 613, in __str__
    assert len(args) >= 2, repr(args)
AssertionError: [EXPR(==[$a1, $a2])]

I have to clue why it happens. Any idea?

[pfalcon]

Any idea?

Bugs, bugs, and more bugs which should be covered by tests ;-). Fixed in 963efd628ca5672a8ced70a2c1ad6fc001e39858 & 994cc6bce39a6f840785751459f7538c92e88959.

[maximumspatium]

Tests have been updated as requested.

[pfalcon]

Cool, merged, thanks!

Happy New Year!

[maximumspatium]

Thanks, many happy returns!