[PATCH] D11861: [IR] Add token types

[Sanjoy Das via llvm-commits]

On Mon, Aug 10, 2015 at 9:10 AM, David Majnemer
<david.majnemer at gmail.com> wrote:
> 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".

Our use case would be around the tokens produced by gc.statepoint.
Currently our scheme is unsound in theory because you could have a phi
of tokens flow into a gc.relocate, and that would be problematic to
lower, at least without code duplication.

With your change, we could return a token_type from a statepoint instead.

-- Sanjoy

>
>
> ================
> 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
>
>
>