Skip to content

Conversation

@Artem-B
Copy link
Member

@Artem-B Artem-B commented Nov 5, 2024

Promote v2i8 to v2i16, fixes a crash.
Re-enable a test in NVPTX/vector-returns.ll

Partial cherry-pick of fda2fea w/o the test which does not exist in release/19.x
#104864

@llvmbot
Copy link
Member

llvmbot commented Nov 5, 2024

@llvm/pr-subscribers-backend-nvptx

Author: Artem Belevich (Artem-B)

Changes

Promote v2i8 to v2i16, fixes a crash.
Re-enable a test in NVPTX/vector-returns.ll

Partial cherry-pick of fda2fea w/o the test which does not exist in release/19.x
#104864


Full diff: https://github.com/llvm/llvm-project/pull/115081.diff

1 Files Affected:

  • (modified) llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp (+4)
diff --git a/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp b/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
index 6975412ce5d35b..b2153a7afe7365 100644
--- a/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
+++ b/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
@@ -229,6 +229,10 @@ static void ComputePTXValueVTs(const TargetLowering &TLI, const DataLayout &DL,
         // v*i8 are formally lowered as v4i8
         EltVT = MVT::v4i8;
         NumElts = (NumElts + 3) / 4;
+      } else if (EltVT.getSimpleVT() == MVT::i8 && NumElts == 2) {
+        // v2i8 is promoted to v2i16
+        NumElts = 1;
+        EltVT = MVT::v2i16;
       }
       for (unsigned j = 0; j != NumElts; ++j) {
         ValueVTs.push_back(EltVT);

@tru
Copy link
Collaborator

tru commented Nov 12, 2024

This seems to have some CI errors, Can you rebase on latest and see if the windows build errors are legit?

@Artem-B
Copy link
Member Author

Artem-B commented Nov 12, 2024

The CI check was unhappy because PR was based on the tree before 19.1.4. I've rebased it on top of the most recent commit.

@tru
Copy link
Collaborator

tru commented Nov 15, 2024

Who can review this?

@Artem-B
Copy link
Member Author

Artem-B commented Nov 15, 2024

That would be me -- I did the review of the original patch and just doing the legwork here to cherry-pick it into the 19.x branch.

If you need somebody else, I'd ask @AlexMaclean

@tru
Copy link
Collaborator

tru commented Nov 19, 2024

Can you back out the merge commit? It needs to be rebased in order to go in cleanly with our scripts.

@Artem-B
Copy link
Member Author

Artem-B commented Nov 19, 2024

Done.

Promote v2i8 to v2i16, fixes a crash.
Re-enable a test in NVPTX/vector-returns.ll

Partial cherry-pick of fda2fea w/o the test which does not exist in release/19.x
llvm#104864
@tru tru merged commit 02930b8 into llvm:release/19.x Nov 25, 2024
15 checks passed
@github-actions
Copy link

@Artem-B (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

@Artem-B
Copy link
Member Author

Artem-B commented Nov 25, 2024

Thank you for merging it. I do not think the fix is interesting enough for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

4 participants