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.

Sign up for GitHub

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

Jump to bottom

[NVPTX] support dynamic allocas with PTX alloca instruction #84585

Merged
merged 2 commits into from
Mar 15, 2024
Merged

[NVPTX] support dynamic allocas with PTX alloca instruction #84585

merged 2 commits into from
Mar 15, 2024

Conversation

AlexMaclean
Copy link
Member

Add support for dynamically sized alloca instructions with the PTX alloca instruction introduced in PTX 7.3 (9.7.15.3. Stack Manipulation Instructions: alloca)

@AlexMaclean AlexMaclean requested a review from Artem-B March 9, 2024 00:04
@elhewaty
Copy link
Member

I was about to work on this issue #62812, I think your patch will fix it.

Artem-B
Artem-B reviewed Mar 12, 2024
View reviewed changes
llvm/lib/Target/NVPTX/NVPTXInstrInfo.td Outdated Show resolved Hide resolved
llvm/lib/Target/NVPTX/NVPTXInstrInfo.td Outdated Show resolved Hide resolved
llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp Show resolved Hide resolved
@AlexMaclean
Copy link
Member Author

I was about to work on this issue #62812, I think your patch will fix it.

I just gave compiling the provided IR a try and I can confirm that with this change it is now successfully compiled (provided a supported arch and PTX version is specified).

@elhewaty
Copy link
Member

That's great. BTW do you have a TODO list I am interested in nvptx but I don't know where to start?

@AlexMaclean
Copy link
Member Author

That's great. BTW do you have a TODO list I am interested in nvptx but I don't know where to start?

@elhewaty, Sorry, I don't personally have anything like this. I'll ask my team, but I don't think NVIDIA has any official list or documentation of this form either. If you have any ideas I'd be happy to give my 2 cents on whether or not I think they make sense to work on.

@AlexMaclean
Copy link
Member Author

@Artem-B, I've address your comments, does this look ready to land now?

@AlexMaclean AlexMaclean merged commit 89b7b3b into llvm:main Mar 15, 2024
4 checks passed
@Artem-B Artem-B mentioned this pull request Jul 8, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Artem-B Artem-B Artem-B approved these changes

Assignees
No one assigned
Labels
backend:NVPTX
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AlexMaclean @elhewaty @Artem-B