Skip to content
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.

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

interpreter: (pdl) fix multiple inserted operations in rewrite pattern interpreter #3569

Open
jianyicheng opened this issue Dec 4, 2024 · 4 comments
Assignees
Labels
bug Something isn't working interpreter xDSL Interpreter

Comments

@jianyicheng
Copy link
Collaborator

jianyicheng commented Dec 4, 2024

(a + b) + c -> a + (b + c)

@superlopuh superlopuh changed the title Minor fix on the base PDL interpreter transformations: fix rewriting in the base PDL interpreter where multiple operations are added in the rewrite body Dec 4, 2024
@superlopuh superlopuh added bug Something isn't working transformations Changes or adds a transformatio labels Dec 4, 2024
@superlopuh superlopuh changed the title transformations: fix rewriting in the base PDL interpreter where multiple operations are added in the rewrite body interpreter: (pdl) fix multiple operations in rewrite pattern interpreter Dec 4, 2024
@superlopuh
Copy link
Member

I would recommend doing a different approach to the one here: https://github.com/xdslproject/xdsl/pull/3402/files#diff-571b0c5cfc943738c6c960434941019e3e0fdc733da7c96dc242bd727e182ff6R402-R415

Instead, accumulating operations created in the pass to a work list, and adding them in the end, instead of getting them from a use-def chain at the end.

@superlopuh superlopuh added interpreter xDSL Interpreter and removed transformations Changes or adds a transformatio labels Dec 4, 2024
@superlopuh superlopuh changed the title interpreter: (pdl) fix multiple operations in rewrite pattern interpreter interpreter: (pdl) fix multiple inserted operations in rewrite pattern interpreter Dec 9, 2024
@superlopuh
Copy link
Member

@cowardsa to provide example

@cowardsa cowardsa self-assigned this Dec 9, 2024
@jumerckx
Copy link
Collaborator

jumerckx commented Feb 4, 2025

builtin.module {
  pdl.pattern : benefit(1) {
    %x = pdl.operand
    %y = pdl.operand
    %z = pdl.operand
    %type = pdl.type
    %accumop = pdl.operation "arith.addi" (%x, %y : !pdl.value, !pdl.value) -> (%type : !pdl.type)
    %accum = pdl.result 0 of %accumop
    %resultop = pdl.operation "arith.addi" (%accum, %z : !pdl.value, !pdl.value) -> (%type : !pdl.type)
    %result = pdl.result 0 of %resultop

    pdl.rewrite %resultop {
      %newaccumop = pdl.operation "arith.addi" (%y, %z : !pdl.value, !pdl.value) -> (%type : !pdl.type)
      %newaccum = pdl.result 0 of %newaccumop

      %newresultop = pdl.operation "arith.addi" (%x, %newaccum : !pdl.value, !pdl.value) -> (%type : !pdl.type)
      %newresult = pdl.result 0 of %newresultop

      pdl.replace %accumop with %newaccumop
      pdl.replace %resultop with %newresultop
    }
  }
  func.func @impl() -> i32 {
    %a = arith.constant 3 : i32
    %a_1 = eqsat.eclass %a : i32
    %b = arith.constant 5 : i32
    %b_1 = eqsat.eclass %b : i32
    %c = arith.constant 7 : i32
    %c_1 = eqsat.eclass %c : i32
    %d = arith.addi %a_1, %b_1 : i32
    %d_1 = eqsat.eclass %d : i32
    %e = arith.addi %d_1, %c_1 : i32
    %e_1 = eqsat.eclass %e : i32
    func.return %e_1 : i32
  }
}

Looking into this issue, is this a test case where a rewrite should occur?

@superlopuh
Copy link
Member

I'm not sure I understand your question, but my understanding is that the expected PDL pattern is only the last line, not the previous one, as the newresult will have the same value as result, and newaccumop will not have the same value as accumop.
Like if you have x = (a + b); y = x + c; y = x + 2, your rewrite would also rewrite x to b + c. But our current interpreter has a bug where the operations that are created in the pdl.rewrite are not inserted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working interpreter xDSL Interpreter
Projects
Development

No branches or pull requests

4 participants