[PATCH] D11861: [IR] Add token types

[Joseph Tremoulet via llvm-commits]

JosephTremoulet added a comment.

I realized I do have an issue to raise as I was going to hook up LLILC to the new EH representation.


================
Comment at: lib/IR/Verifier.cpp:2200
@@ +2199,3 @@
+  // Check that a PHI doesn't yield a Token.
+  Assert(!PN.getType()->isTokenTy(), "PHI nodes cannot have token type!");
+
----------------
I think we'll need to allow token PHIs with undef, if we want to use this for EH pads and we still want code commoning in EH pads to be legal.

I.e., if we have something along the lines of
```
cleanup1:
  cleanuppad
  ; chunk of code
  cleanupret unwind label %next1
cleanup2:
  cleanuppad
  ; identical chunk of code
  cleanupret unwind label %next2
```
then it was my understanding that it was a key design point of the new EH representation that it would be legal (if unprofitable) to transform this to
```
cleanup1:
  cleanuppad
  br label %shared_chunk
tail1:
  cleanupret unwind label %next1
shared_chunk:
  %origin = phi i1 [ 0, %cleanup1 ], [ 1, %cleanup2 ]
  ; chunk of code
  br i1 %origin, label %tail1, label %tail2
cleanup2:
  cleanuppad
  br label %shared_chunk
tail2:
  cleanupret unwind label %next2
```

but if cleanuppad produces a token that cleanupret consumes:
```
cleanup1:
  %tok1 = cleanuppad
  ; chunk of code
  cleanupret token %tok1, unwind label %next1
cleanup2:
  %tok2 = cleanuppad
  ; identical chunk of code
  cleanupret token %tok2, unwind label %next2
```
then a blanket rule against token PHIs precludes the transformation (the token defs would no longer dominate the uses).  On the other hand, if we allow PHI with undef (as such a transformation would naturally introduce for any SSA values live across the combined regions), we could create this:
```
cleanup1:
  %tok1 = cleanuppad
  br label %shared_chunk
tail1:
  cleanupret token %tok1_copy, unwind label %next1
shared_chunk:
  %origin = phi i1 [ 0, %cleanup1 ], [ 1, %cleanup2 ]
  %tok1_copy = phi token [ %tok1, %cleanup1 ], [ undef, %cleanup2 ]
  %tok2_copy = phi token [ undef, %cleanup1 ], [ %tok2, %cleanup2 ]
  ; chunk of code
  br i1 %origin, label %tail1, label %tail2
cleanup2:
  %tok2 = cleanuppad
  br label %shared_chunk
tail2:
  cleanupret token %tok2_copy, unwind label %next2
```

(on the EH side: if things have changed and we're ok with disallowing this transformation, that's fine with me but it puts cleanupcall on my critical path)

That said, there are different things that "PHI with undef" could mean:
 1) allow a token PHI with only one non-undef incoming value ( %x = phi token [ %y, %1 ], [ undef, %2 ], [ undef, %3 ] )
 2) allow a token PHI with only one distinct non-undef incoming value ( %x = phi token [ %y, %1 ], [ %y, %2 ], [ undef, %3 ] )
 3) also allow directly recursive token PHIs ( %x = phi token [ %y, %1 ], [ %x, %2] , [ undef, %3] )
 4) others?
 5) allow a web of token PHIs so long as there's only one unique incoming value, discounting undef and other phis in the web

#2 seems like a reasonable starting place to me, though any of them would be good enough for my immediate intended purpose (which is to encode finalies with this construct per Reid's suggestion (http://article.gmane.org/gmane.comp.compilers.llvm.devel/85937) as a staging plan until we add cleanupcall).


http://reviews.llvm.org/D11861