Skip to content

Enhance the logic of phi -> phi_start transform#295

Open
ShangkunLi wants to merge 3 commits intocoredac:mainfrom
ShangkunLi:phi_start_debug
Open

Enhance the logic of phi -> phi_start transform#295
ShangkunLi wants to merge 3 commits intocoredac:mainfrom
ShangkunLi:phi_start_debug

Conversation

@ShangkunLi
Copy link
Collaborator

In this pr,

  • We enhance the logic of transforming phi to phi_start. In the previous logic, we transform all phi with a reserve op as an operand into phi_start. This may violate the semantics in the simulator, so we only transform phi ops with a reserve op + a grant_once op into phi_start ops
  • We modify the codegen pass for the corresponding changes

@ShangkunLi
Copy link
Collaborator Author

Hi~ @n0thingNoob,

Since you can handle the phi that has two operands with the false predicate, do we still need phi_start now?

@ShangkunLi
Copy link
Collaborator Author

Hi~ @n0thingNoob,

Since you can handle the phi that has two operands with the false predicate, do we still need phi_start now?

After discussion with @n0thingNoob, we can handle this later after evaluation of their project.

@tancheng
Copy link
Contributor

  • so we only transform phi ops with a reserve op + a grant_once op into phi_start ops

What we avoid transforming using this PR?

@ShangkunLi
Copy link
Collaborator Author

  • so we only transform phi ops with a reserve op + a grant_once op into phi_start ops

What we avoid transforming using this PR?

For ir like:

    %48 = neura.grant_predicate %39, %43 : !neura.data<i64, i1>, !neura.data<i1, i1> -> !neura.data<i64, i1>
    %49 = neura.grant_predicate %39, %43 : !neura.data<i64, i1>, !neura.data<i1, i1> -> !neura.data<i64, i1>
    %50 = neura.grant_predicate %40, %43 : !neura.data<i32, i1>, !neura.data<i1, i1> -> !neura.data<i32, i1>
    %51 = neura.reserve : !neura.data<i32, i1>
    %52 = neura.phi_start %50, %51 : !neura.data<i32, i1>, !neura.data<i32, i1> -> !neura.data<i32, i1>
    %53 = neura.reserve : !neura.data<i64, i1>
    %54 = neura.phi_start %49, %53 : !neura.data<i64, i1>, !neura.data<i64, i1> -> !neura.data<i64, i1>
    %55 = neura.reserve : !neura.data<i64, i1>
    %56 = neura.phi_start %48, %55 : !neura.data<i64, i1>, !neura.data<i64, i1> -> !neura.data<i64, i1>
    %57 = "neura.gep"(%56) <{operandSegmentSizes = array<i32: 0, 1>}> {lhs_value = "%arg3"} : (!neura.data<i64, i1>) -> !neura.data<!llvm.ptr, i1>
    %58 = "neura.load"(%57) : (!neura.data<!llvm.ptr, i1>) -> !neura.data<i32, i1>
    %59 = "neura.icmp"(%58) <{cmpType = "slt"}> {rhs_value = 0 : i32} : (!neura.data<i32, i1>) -> !neura.data<i1, i1>
    %60 = neura.grant_predicate %57, %59 : !neura.data<!llvm.ptr, i1>, !neura.data<i1, i1> -> !neura.data<!llvm.ptr, i1>
    %61 = neura.grant_predicate %54, %59 : !neura.data<i64, i1>, !neura.data<i1, i1> -> !neura.data<i64, i1>
    %62 = neura.grant_predicate %52, %59 : !neura.data<i32, i1>, !neura.data<i1, i1> -> !neura.data<i32, i1>
    %63 = "neura.not"(%59) : (!neura.data<i1, i1>) -> !neura.data<i1, i1>
    %64 = neura.grant_predicate %54, %63 : !neura.data<i64, i1>, !neura.data<i1, i1> -> !neura.data<i64, i1>
    %65 = neura.grant_predicate %52, %63 : !neura.data<i32, i1>, !neura.data<i1, i1> -> !neura.data<i32, i1>

These phi_start don't have any operand with a true predicate when they are executed for the first time. So they actually have the same semantics as phi. I think we should clean this up.

@tancheng
Copy link
Contributor

Okay, so it sounds like, if the LHS is neura.grant_predicate, we won't transform it to phi_start; if LHS is neura.grant_once, we do the transformation.

Above summary is correct?

@ShangkunLi
Copy link
Collaborator Author

Okay, so it sounds like, if the LHS is neura.grant_predicate, we won't transform it to phi_start; if LHS is neura.grant_once, we do the transformation.

Above summary is correct?

Yes, this is what this pr done.

But after carefully thinking, I think we don't even need a phi_start. Because if we encounter:

  1. phi (<val, 1>, <N/A, N/A>) or phi (<N/A, N/A>, <val, 1>): we should simply choose the one with predicate 1
  2. phi (<N/A>, <N/A>): we should output <AnyVal, 0>

The semantics of phi is robust enough, I think.

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