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

Add speculatable attribute #109

Merged
merged 1 commit into from
Oct 17, 2024
Merged

Conversation

vchuravy
Copy link
Contributor

Noticed that this was missing while experimenting with llvm-dialects.

Is there already an interface for specifing return attributes / per-argument interfaces?

@nhaehnle
Copy link
Member

Thanks!

Is there already an interface for specifing return attributes / per-argument interfaces?

No. Look how llvm_dialects::genDialectDefs invokes LlvmAttributeTrait::addAttribute to generate the code that adds attributes to attribute lists. We'd have to extend both the invocation site to support adding attributes at indices other than FunctionIndex, and the Trait hierarchy to represent attributes in argument or return position.

Perhaps an LlvmArgEnumAttributeTrait class in TableGen that allows defining

def NoCaptureArg<int idx> : LlvmArgEnumAttributeTrait<idx, "NoCapture">;
// etc.

and similar for return attributes.

@nhaehnle nhaehnle merged commit a3f36a8 into GPUOpen-Drivers:dev Oct 17, 2024
8 checks passed
@nhaehnle
Copy link
Member

Thinking about this a bit more, working with indices when defining operations is a bit clunky. Perhaps the TableGen should look like this:

def FooOp : ... {
  let arguments = (ins Ptr:$ptr);
  let results = (outs Ptr:$result);

  let value_traits = [
    (ReadOnly $ptr),
    (NoCapture $ptr),
    (NonNull $result),
  ];
}

We could then potentially re-use the LlvmEnumAttributeTrait TableGen class, but perhaps add a field that indicates which positions the attribute can appear in.

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