bril icon indicating copy to clipboard operation
bril copied to clipboard

[SSA] Fix up to_ssa.py and add test cases

Open terrelln opened this issue 4 years ago • 3 comments

This PR contains 4 commits detailed below. In summary the goal is to remove all uses of undefined variables. It fixes all the problems of Issue #108. It is done in 3 steps:

  1. Add support for null pointers to brili and brilirs, created with alloc 0.
  2. Fix a bug in rename() that caused references to undefined variables (not __undefined) to be emitted.
  3. Define __undefined.${type} for each type at the top of the entry block, and use __undefined.${type} instead of __undefined.

After these steps phi nodes no longer have undefined arguments, and no longer produce undefined results. The ability to execute phi nodes with undefined or missing arguments could be removed from brili, or hidden behind a flag (but I haven't done that).

I added one test case to_ssa/ called loop-branch.bril which has an interesting feature where two phis form a dependency loop: v.0 depends on v.1 and visa-versa.

Then I add test cases for from_ssa.py that executes the round-tripped code for every test in to_ssa/. I also tested on all the programs in the benchmarks directory, and it works on everything in that directory. I haven't added them to the from_ssa tests to avoid adding overly complex tests.

This newly generated code inserts potentially unnecessary __undefined.${type} definitions. A DCE pass can trivially remove the unnecessary ones.


[brili][memory] Support null pointers

  • Null pointers are created by alloc 0.
  • All null pointers share the same key Key(0, 0).
  • Freeing a null pointer is a no-op.
  • Null pointers do not need to be freed.
  • Reading/writing a null pointer is an error.
  • Pointer addition is allowed on null pointers.

[to_ssa] Fix bug in rename()

The use of defaultdict() and stack.update(old_stack) meant that any stacks that were empty at the top of _rename() weren't cleared at the bottom. This caused use of undefined variables inserted into phi instead of __undefined.


[to_ssa] Define undefined variables

Break undefined variables up by type, to meet the bril spec that each variable has only one type. Then, define the undefined variable for each type in the entry block. This ensures that no phi node uses undefined variables, or produces undefined variables, in any case.

Add an interesting test case loop-branch.bril which produces phi nodes that form a dependency loop.


[from_ssa] Add test cases

Add brili test cases, since I want to test that no undefined varaibles are used during execution after from_ssa.

terrelln avatar Jan 12 '21 04:01 terrelln

I think the allowance of undefined variables in phi nodes may have been a mistake. The wrinkle that phi nodes are allowed undefined values, but no other instruction is, really complicates analysis of the bril language.

I think that there are two logical choices, and anything in the middle will cause headaches:

  1. Strictly allow no undefined variables.
  2. Allow undefined variables in all ops. Any operation with undefined variables is undefined.

terrelln avatar Jan 12 '21 04:01 terrelln

Cool! This is probably much cleaner than my approach. The end result is the same, having to define an __undefine. Couldn't this be removed with a pass before emitting code in SSA form (as you mentioned earlier)?

cgyurgyik avatar Jan 12 '21 12:01 cgyurgyik

I've added support for the memory extension to to_ssa.py. In order to do that, I needed to support null pointers in brili and brilirs. Null pointers are created with alloc 0, and do not need to be freed. See the first commit message for details.

I've decided to use alloc 0, so as not to introduce another instruction, or make const memory aware.

terrelln avatar Jan 16 '21 18:01 terrelln