[PATCH] D11861: [IR] Add token types

[David Majnemer via llvm-commits]

majnemer added a comment.

In http://reviews.llvm.org/D11861#220538, @rnk wrote:

> Can you expand on the use cases for tokens? It sounded like there were more than just this on Friday.
>
> Fundamentally, this is breaking the invariant that values produced by instructions Can Be Phied. We need more than one use case to want to break that rule.


The folks working on safepoints need something along these lines in order to correctly track pointer provenance.  @sanjoy or @reames could probably do a better job describing the role of token types in "argument bundles".


================
Comment at: lib/IR/Type.cpp:383
@@ -380,3 +382,3 @@
   return !RetTy->isFunctionTy() && !RetTy->isLabelTy() &&
-  !RetTy->isMetadataTy();
+         !RetTy->isMetadataTy() && !RetTy->isTokenTy();
 }
----------------
JosephTremoulet wrote:
> Will this preclude having intrinsics that produce tokens?  I'd have thought such a thing would make sense, though I don't have an immediate need for one.
We can always loosen the restriction later if we need to.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:1644-1645
@@ -1643,1 +1643,4 @@
   for (BasicBlock::const_iterator I = BB->begin(), E = BB->end(); I != E; ++I) {
+    if (I->getType()->isTokenTy())
+      return true;
+
----------------
rnk wrote:
> Shouldn't this have a test?
This change was made by inspection.  I'll try to come up with a test case.

================
Comment at: test/Assembler/token.ll:6
@@ +5,3 @@
+; CHECK: define void @sh16(token
+define void @sh16(token %A) {
+entry:
----------------
JosephTremoulet wrote:
> Seems odd to allow token parameters but not token returns; I'd have expected each kind of cross-function transfer to cause roughly the same amount of trouble.
Right, I'll preclude this possibility.

================
Comment at: test/Verifier/token1.ll:8
@@ +7,3 @@
+bb:
+  %phi = phi token [ %A, %bb ], [ %B, %entry]
+; CHECK: PHI nodes cannot have token type!
----------------
rnk wrote:
> I think you can combine these tests into one token-invalid.ll file. You get one verifier failure per function, so if you keep them split up like this, it'll work.
Sure, will do.


http://reviews.llvm.org/D11861