Skip to content

Canonicalization: Remove unused primal arguments for region op#2640

Merged
vimarsh6739 merged 1 commit intomainfrom
arg-canonicalization
Jan 27, 2026
Merged

Canonicalization: Remove unused primal arguments for region op#2640
vimarsh6739 merged 1 commit intomainfrom
arg-canonicalization

Conversation

@vimarsh6739
Copy link
Copy Markdown
Member

It is safe to remove unused primal arguments for AutoDiffRegionOp when the primal has no uses within the function and is enzyme_const

Copy link
Copy Markdown
Member

@wsmoses wsmoses left a comment

Choose a reason for hiding this comment

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

It's safe to remove any unused arg, not just const

@vimarsh6739
Copy link
Copy Markdown
Member Author

There is one caveat, we assume that the only use of primals in the region are the block arguments of the entry basic block(and not the passed arguments to the op). Should we add a verifier for this? Currently, this is a manual check which is repeated for both outlining and #2562

@wsmoses
Copy link
Copy Markdown
Member

wsmoses commented Dec 23, 2025

e assume that the only use of primals in the region are the block arguments of the entry basic block(and not the passed arguments to the op). Should we add a verifier for this? Currently, this is a manual check which is repeated for both out

No that's a limitation that should be fixed by the requisite users, as that is legal IR that ought be supported [tho perhaps not as well optimized]

Comment thread enzyme/Enzyme/MLIR/Dialect/Ops.cpp Outdated
Comment thread enzyme/Enzyme/MLIR/Dialect/Ops.cpp
@vimarsh6739 vimarsh6739 force-pushed the arg-canonicalization branch 3 times, most recently from f7dfb69 to 4021d22 Compare December 24, 2025 11:03
@vimarsh6739
Copy link
Copy Markdown
Member Author

vimarsh6739 commented Dec 24, 2025

@wsmoses thinking a bit more about it, the trigger conditions for args with activity which aren't enzyme_const is dicey. Consider the following example:

func.func @foo(%x : f64, %dx : f64) -> f64 {
  %cst2 = arith.constant 2.0 : f64
  %x2 = arith.mulf %cst2, %x : f64
  %dout = enzyme.fwddiff_region(%x, %dx){
  ^bb0(%xx : f64):
      %y = arith.mulf %x2, %x2 : f64
      enzyme.yield %y : f64 
  } attributes { activity = [#enzyme<activity enzyme_dup>], ret_activity=[#enzyme<activity enzyme_dupnoneed>] }: (f64,f64) -> f64
  return %dout : f64
}

If we remove (%xx == {%x, %dx}) (since it is never used, we would run into issues, since %x2 is effectively re-added always as enzyme_const when outlining the region and lowering).

This also holds for the reverse mode case(in the below example, %x2 will be considered enzyme_const)

func.func @foo(%x : f64, %dout : f64) -> f64 {
  %cst2 = arith.constant 2.0 : f64
  %x2 = arith.mulf %cst2, %x : f64
  %dx = enzyme.autodiff_region(%x, %dout){
  ^bb0(%xx : f64):
      %y = arith.mulf %x2, %x2 : f64
      enzyme.yield %y : f64 
  } attributes { activity = [#enzyme<activity enzyme_active>], ret_activity=[#enzyme<activity enzyme_activenoneed>] }: (f64,f64) -> f64
  return %dx : f64
}

The other option is to leave it as a convention and make a note somewhere to users that this would const out computations.

@vimarsh6739 vimarsh6739 changed the title Canonicalization: Remove unused primal arguments for AutoDiffRegionOp Canonicalization: Remove unused primal arguments for region op Dec 24, 2025
@wsmoses
Copy link
Copy Markdown
Member

wsmoses commented Dec 27, 2025

ove (%xx == {%x, %dx}) (since it is never used, we would run into issues, since %x2 is effectively re-added always a

I don't understand this case, x2 is always const here. in fact the entire region is constant? %xx is literally unused in the region so it is always safe to remove, definitionally.

It is safe to remove unused primal arguments for AutoDiffRegionOp when
the primal has no uses within the function.
@vimarsh6739 vimarsh6739 added this pull request to the merge queue Jan 27, 2026
Merged via the queue into main with commit 15a3d98 Jan 27, 2026
27 of 29 checks passed
@vimarsh6739 vimarsh6739 deleted the arg-canonicalization branch January 27, 2026 18:31
vimarsh6739 added a commit that referenced this pull request Jan 27, 2026
@vimarsh6739 vimarsh6739 restored the arg-canonicalization branch January 27, 2026 23:20
vimarsh6739 added a commit that referenced this pull request Mar 17, 2026
vimarsh6739 added a commit that referenced this pull request Mar 17, 2026
wsmoses pushed a commit that referenced this pull request Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants