Skip to content

call.without.effects is always possible for CSE #7568

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

call.without.effects is always possible for CSE #7568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eafc986
call.without.effects is always possible for CSE
paul-hoang May 3, 2025
9eee2aa
Update LocalCSE.cpp
paul-hoang May 3, 2025
b7fbfc1
add test
paul-hoang May 3, 2025
3303f7f
add checks
paul-hoang May 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/passes/LocalCSE.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@

#include <ir/cost.h>
#include <ir/effects.h>
#include <ir/intrinsics.h>
#include <ir/iteration.h>
#include <ir/linear-execution.h>
#include <ir/properties.h>
Expand Down Expand Up @@ -363,6 +364,11 @@ struct Scanner
// total size big enough - while isPossible checks conditions that prevent
// using an expression at all.
bool isPossible(Expression* curr) {
// call.without.effects is always possible for CSE.
if (Intrinsics(*getModule()).isCallWithoutEffects(curr)) {
return true;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, isn't the isGenerative check at the end of the function still important? For example, a call.without.effects might return a random number on each call - that has no effects, but we can't CSE it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought returning a random number is considered an effect. Meaning if a function return random number, it should not be called/marked as "call.without.effect".

But if I'm misunderstanding, should we then introduce "call.pure"? So that it can be applied to pure function call and take benefit from CSE.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effects are the things we consider in effects.h, see the properties there (like reading memory, throwing, etc.).

Something like generating random numbers, but having no other side effects, is covered under what we call "generativity" (generating unexpected/different values). That in theory could have been part of effects.h, but it is rare enough to matter that we left it separate.

So something like "pure" calls would be needed to optimize here. Actually I am planning to rework the intrinsics soon anyhow, avoiding the awkward import we use atm, in favor of something more like the Branch Hinting and Compilation Hints proposals. I am adding Branch Hinting now, so once the shared infrastructure is ready, we can move this to there. And I think we may want to rename it "pure" at that point, and include generativity - does that sound good?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, including generativity in to "pure" annotation/hint sounds good.

It's indeed awkward to apply "call.without.effects" in the current way.. It might be a breaking change (since we currently use "call.without.effects" in the current way) but sound be easy enough to migrate.

For this PR, I think you can close it now if you want. If possible, please cc me in the issue/PR you planned for reworking the intrinsics.


// We will fully compute effects later, but consider shallow effects at this
// early time to ignore things that cannot be optimized later, because we
// use a greedy algorithm. Specifically, imagine we see this:
Expand Down
59 changes: 59 additions & 0 deletions test/lit/passes/local-cse_call-without-effects.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.
;; RUN: foreach %s %t wasm-opt --local-cse --intrinsic-lowering -all -S -o - | filecheck %s

(module
;; CHECK: (import "binaryen-intrinsics" "call.without.effects" (func $cwe-ii-i (type $1) (param i32 i32 funcref) (result i32)))
(import "binaryen-intrinsics" "call.without.effects" (func $cwe-ii-i (param i32 i32 funcref) (result i32)))

;; CHECK: (func $add (type $0) (param $0 i32) (param $1 i32) (result i32)
;; CHECK-NEXT: (i32.add
;; CHECK-NEXT: (local.get $0)
;; CHECK-NEXT: (local.get $1)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $add (param $0 i32) (param $1 i32) (result i32)
(i32.add (local.get $0) (local.get $1))
)

;; CHECK: (func $test (type $0) (param $0 i32) (param $1 i32) (result i32)
;; CHECK-NEXT: (local $2 i32)
;; CHECK-NEXT: (local $3 i32)
;; CHECK-NEXT: (local $4 i32)
;; CHECK-NEXT: (local.set $2
;; CHECK-NEXT: (local.tee $4
;; CHECK-NEXT: (call $add
;; CHECK-NEXT: (local.get $0)
;; CHECK-NEXT: (local.get $1)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $3
;; CHECK-NEXT: (local.get $4)
;; CHECK-NEXT: )
;; CHECK-NEXT: (return
;; CHECK-NEXT: (i32.add
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $test (param $0 i32) (param $1 i32) (result i32)
(local $2 i32)
(local $3 i32)
(local.set $2
(call $cwe-ii-i
(local.get $0)
(local.get $1)
(ref.func $add)
)
)
(local.set $3
(call $cwe-ii-i
(local.get $0)
(local.get $1)
(ref.func $add)
)
)
(return (i32.add (local.get $2) (local.get $3)))
)
)