Grand Unified Flow Analysis (GUFA) #4598

kripken · 2022-04-18T23:03:27Z

This is far from ready for review, but @tlively was curious to see the current status, so posting.

This tracks the possible contents in the entire program all at once using a single IR. That is in contrast to say DeadArgumentElimination of LocalRefining etc., all of whom look at one particular aspect of the program (function params and returns in DAE, locals in LocalRefining). The cost is to build up an entire new IR, which takes a lot of code - ~2000 lines atm, but should be close to done. At least all that code is separable from everything else and could fit entirely in the new pass. Another cost is this new IR is very big and requires a lot of time and memory to process. The benefit is that this can find opportunities that are only obvious when looking at the entire program, and also it can track information that is more specialized than the normal type system in the IR - in particular, this can track an ExactType, which is the case where we know the value is of a particular type exactly and not a subtype. Both may end up useful, but it's too early to tell.

This passes fuzzing (but we don't fuzz --nominal well atm, so that is somewhat limited) and a large amount of new tests, but removes too much code in dart2wasm and breaks things there. It is also too large/slow to run on j2wasm atm.

edit: now passes on dart2wasm. it removes 2% of code size and 5% of vars.

edit edit: This speeds up j2wasm microbenchmarks by 20%, and is ready for review.

I think we can actually move all the checking routines to `getDroppedUnconditionalChildrenAndAppend` and remove `canRemove` and `canRemoveStructurally`. Checking things in three different places was a little hard to understand, and I think `getDroppedUnconditionalChildrenAndAppend` actually needs to exclude more things that `canRemoveStructurally` covers, if it is to be used in other places as well. Additionally, I think it'd better to make these two dropping-children functions (`getDroppedChildrenAndAppend` and `getDroppedUnconditionalChildrenAndAppend`) to a cpp file and make `getDroppedChildrenAndAppend` an internal function, inaccessible to outer passes, given that it looks all places should use `getDroppedUnconditionalChildrenAndAppend` instead. But this can be a follow-up. Not sure why the test signatures change. May need to investigate..?

test/lit/passes/gufa-refs.wast

aheejin · 2022-07-11T10:11:30Z

test/lit/passes/gufa-vs-cfp.wast

+  (func $test
+    ;; The only place this type is created is with a default value, and so we
+    ;; can optimize the get into a constant (note that no drop of the
+    ;; ref is needed: the optimizer can see that the struct.get cannot trap, as
+    ;; its reference is non-nullable).
+    (drop
+      (struct.get $struct 0
+        (struct.new_default_with_rtt $struct
+          (rtt.canon $struct)
+        )
+      )
+    )
+  )


The function body is different from the counterpart from cfp.wast:

binaryen/test/lit/passes/cfp.wast

Lines 55 to 74 in 44fa122

(func $test

;; The only place this type is created is with a default value, and so we

;; can optimize the later get into a constant (plus a drop of the ref).

;;

;; (Note that the allocated reference is dropped here, so it is not actually

;; used anywhere, but this pass does not attempt to trace the paths of

;; references escaping and being stored etc. - it just thinks at the type

;; level.)

(drop

(struct.new_default_with_rtt $struct

(rtt.canon $struct)

)

)

(drop

(struct.get $struct 0

(ref.null $struct)

)

)

)

)

Also there are more different functions between the two. The GUFA's version seems to be a condensed version of CFP's. Is this intentional?

The specific reason in the test you mentioned is that dropping each result leads to GUFA seeing that the result does not reach anywhere. And that null is an opaque placeholder for CFP, but GUFA sees that it will trap. So we need to connect results to uses, which "compresses" the test.

More details here:

binaryen/test/lit/passes/gufa-vs-cfp.wast

Lines 6 to 22 in adef90c

;; This is almost identical to cfp.wast, and is meant to facilitate comparisons

;; between the passes - in particular, gufa should do everything cfp can do,

;; although it may do it differently. Changes include:

;;

;; * Tests must avoid things gufa optimizes away that would make the test

;; irrelevant. In particular, parameters to functions that are never called

;; will be turned to unreachable by gufa, so instead make those calls to

;; imports. Gufa will also realize that passing ref.null as the reference of

;; a struct.get/set will trap, so we must actually allocate something.

;; * Gufa optimizes in a more general way. Cfp will turn a struct.get whose

;; value it infers into a ref.as_non_null (to preserve the trap if the ref is

;; null) followed by the constant. Gufa has no special handling for

;; struct.get, so it will use its normal pattern there, of a drop of the

;; struct.get followed by the constant. (Other passes can remove the

;; dropped operation, like vacuum in trapsNeverHappen mode).

;; * Gufa's more general optimizations can remove more unreachable code, as it

;; checks for effects (and removes effectless code).

aheejin

Very impressive framework! 😮 LGTM and sorry for the super delayed review! 😅

This reverts commit e2ce69c.

This reverts commit 5aa2e18.

This reverts commit c6c0769.

…)"" This reverts commit c00db9e.

This reverts commit 7988682, reversing changes made to 0da2d8c.

This reverts commit d556760.

kripken · 2022-07-12T16:46:18Z

Thanks for the thorough review @aheejin !

I think we can actually move all the checking routines to `getDroppedUnconditionalChildrenAndAppend` and remove `canRemove` and `canRemoveStructurally`. Checking things in three different places was a little hard to understand, and I think `getDroppedUnconditionalChildrenAndAppend` actually needs to exclude more things that `canRemoveStructurally` covers, if it is to be used in other places as well. Additionally, I think it'd better to make these two dropping-children functions (`getDroppedChildrenAndAppend` and `getDroppedUnconditionalChildrenAndAppend`) to a cpp file and make `getDroppedChildrenAndAppend` an internal function, inaccessible to outer passes, given that it looks all places should use `getDroppedUnconditionalChildrenAndAppend` instead. But this can be a follow-up.

aheejin · 2022-07-18T22:26:34Z

Umm, I think I messed up something while merging...

This reverts commit 5198ccc, reversing changes made to 5e3d67c.

I think we can actually move all the checking routines to `getDroppedUnconditionalChildrenAndAppend` and remove `canRemove` and `canRemoveStructurally`. Checking things in three different places was a little hard to understand, and I think `getDroppedUnconditionalChildrenAndAppend` actually needs to exclude more things that `canRemoveStructurally` covers, if it is to be used in other places as well. Additionally, I think it'd better to make these two dropping-children functions (`getDroppedChildrenAndAppend` and `getDroppedUnconditionalChildrenAndAppend`) to a cpp file and make `getDroppedChildrenAndAppend` an internal function, inaccessible to outer passes, given that it looks all places should use `getDroppedUnconditionalChildrenAndAppend` instead. But this can be a follow-up.

aheejin · 2022-07-18T22:34:37Z

Sorry, I tried to do merge #4787 but forgot to squash and just did merge from the command line, which dragged in all commits from #4787, which I messed up separately (and managed to restore)... Anyway I reverted the merge and manually applied the diff from #4787.

kripken · 2022-07-18T22:48:20Z

Thanks @aheejin !

kripken · 2022-07-20T15:00:02Z

Friendly ping @tlively , did you want to take a look here?

tlively · 2022-07-21T18:01:17Z

Thanks for the ping. I'll take a look today.

tlively

Half-rubberstamp LGTM. I don't want to hold this up any longer 😞

tlively · 2022-06-30T03:50:26Z

test/lit/passes/gufa-no_names.wast

+;; Two tags with different values. Names are added by text format parsing, which
+;; would inhibit optimizations, hence this pass requires unused names to be
+;; removed.


How can names inhibit optimizations?

(This was a very old pending comment, feel free to ignore)

kripken added 30 commits May 18, 2022 14:22

Merge remote-tracking branch 'origin/main' into fgprop

4f1a6b1

clean

52535db

polish

3a4a894

polish

c3e3e30

polish

8dfedaa

polish

5457df6

better

9a2e636

rename

d18b933

format

13adac9

work

adf9227

fix

aecfd18

fix

eee2e30

format

0444b41

test

bcaea49

text

2270174

text

9d01a3b

work

0e496a9

text

f620b76

text

b74f427

text

e0d0592

text

4627648

text

265c6f2

text

152541f

text

6faaa19

text

34380e0

text

7f07af0

text

732d881

fix

7c73a1f

test

243a960

test

e304cdf

aheejin added 3 commits July 8, 2022 22:31

Restore comments

3704951

clang-format

0da2d8c

aheejin reviewed Jul 11, 2022

View reviewed changes

feedback

68fcea1

aheejin approved these changes Jul 12, 2022

View reviewed changes

aheejin and others added 12 commits July 12, 2022 00:32

4598

483087c

Merge branch '4598' into improve_remove

7988682

Revert "Fix binaryen.js to include allocate() explicitly (#4793)"

c00db9e

This reverts commit e2ce69c.

Revert "[Parser] Start to parse instructions (#4789)"

c6c0769

This reverts commit 5aa2e18.

Revert "Revert "[Parser] Start to parse instructions (#4789)""

8f9e2a6

This reverts commit c6c0769.

Revert "Revert "Fix binaryen.js to include allocate() explicitly (#4793…

8955856

…)"" This reverts commit c00db9e.

Revert "Merge branch '4598' into improve_remove"

1eed7e0

This reverts commit 7988682, reversing changes made to 0da2d8c.

feedback

d556760

Revert "feedback"

14d83ef

This reverts commit d556760.

Add optimize = trues

4a1ca02

Test changes

8925759

Merge remote-tracking branch 'origin/main' into gufa

5e3d67c

aheejin added 2 commits July 18, 2022 15:28

Revert "Simplify routines for dropping children"

cd8aa1e

This reverts commit 5198ccc, reversing changes made to 5e3d67c.

tlively approved these changes Jul 22, 2022

View reviewed changes

kripken merged commit ed70444 into main Jul 22, 2022

kripken deleted the gufa branch July 22, 2022 15:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Grand Unified Flow Analysis (GUFA) #4598

Grand Unified Flow Analysis (GUFA) #4598

kripken commented Apr 18, 2022 •

edited

Loading

aheejin Jul 11, 2022

kripken Jul 11, 2022

aheejin left a comment

kripken commented Jul 12, 2022

aheejin commented Jul 18, 2022

aheejin commented Jul 18, 2022

kripken commented Jul 18, 2022

kripken commented Jul 20, 2022

tlively commented Jul 21, 2022

tlively left a comment

tlively Jun 30, 2022

tlively Jul 22, 2022

	(func $test
	;; The only place this type is created is with a default value, and so we
	;; can optimize the later get into a constant (plus a drop of the ref).
	;;
	;; (Note that the allocated reference is dropped here, so it is not actually
	;; used anywhere, but this pass does not attempt to trace the paths of
	;; references escaping and being stored etc. - it just thinks at the type
	;; level.)
	(drop
	(struct.new_default_with_rtt $struct
	(rtt.canon $struct)
	)
	)
	(drop
	(struct.get $struct 0
	(ref.null $struct)
	)
	)
	)
	)

	;; This is almost identical to cfp.wast, and is meant to facilitate comparisons
	;; between the passes - in particular, gufa should do everything cfp can do,
	;; although it may do it differently. Changes include:
	;;
	;; * Tests must avoid things gufa optimizes away that would make the test
	;; irrelevant. In particular, parameters to functions that are never called
	;; will be turned to unreachable by gufa, so instead make those calls to
	;; imports. Gufa will also realize that passing ref.null as the reference of
	;; a struct.get/set will trap, so we must actually allocate something.
	;; * Gufa optimizes in a more general way. Cfp will turn a struct.get whose
	;; value it infers into a ref.as_non_null (to preserve the trap if the ref is
	;; null) followed by the constant. Gufa has no special handling for
	;; struct.get, so it will use its normal pattern there, of a drop of the
	;; struct.get followed by the constant. (Other passes can remove the
	;; dropped operation, like vacuum in trapsNeverHappen mode).
	;; * Gufa's more general optimizations can remove more unreachable code, as it
	;; checks for effects (and removes effectless code).

Grand Unified Flow Analysis (GUFA) #4598

Grand Unified Flow Analysis (GUFA) #4598

Conversation

kripken commented Apr 18, 2022 • edited Loading

aheejin Jul 11, 2022

Choose a reason for hiding this comment

kripken Jul 11, 2022

Choose a reason for hiding this comment

aheejin left a comment

Choose a reason for hiding this comment

kripken commented Jul 12, 2022

aheejin commented Jul 18, 2022

aheejin commented Jul 18, 2022

kripken commented Jul 18, 2022

kripken commented Jul 20, 2022

tlively commented Jul 21, 2022

tlively left a comment

Choose a reason for hiding this comment

tlively Jun 30, 2022

Choose a reason for hiding this comment

tlively Jul 22, 2022

Choose a reason for hiding this comment

kripken commented Apr 18, 2022 •

edited

Loading