Skip to content

Clang fails when building LLVM for RISCV #81088

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

Closed
kongy opened this issue Feb 8, 2024 · 5 comments · Fixed by #81107
Closed

Clang fails when building LLVM for RISCV #81088

kongy opened this issue Feb 8, 2024 · 5 comments · Fixed by #81107

Comments

@kongy
Copy link
Collaborator

kongy commented Feb 8, 2024

fatal error: error in backend: Do not know how to split the result of this operator!

Reduced reproducer attached.

reduce.zip

@github-actions github-actions bot added the clang Clang issues not falling into any other category label Feb 8, 2024
@EugeneZelenko EugeneZelenko added backend:RISC-V and removed clang Clang issues not falling into any other category labels Feb 8, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 8, 2024

@llvm/issue-subscribers-backend-risc-v

Author: Yi Kong (kongy)

`fatal error: error in backend: Do not know how to split the result of this operator!`

Reduced reproducer attached.

reduce.zip

@topperc
Copy link
Collaborator

topperc commented Feb 8, 2024

Looks like #66800 is missing a check for legal type.

@topperc topperc assigned luke957 and lukel97 and unassigned luke957 Feb 8, 2024
@topperc
Copy link
Collaborator

topperc commented Feb 8, 2024

Possible fix

diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 27037f4d5c5c..405d99f14d8f 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -15812,7 +15812,8 @@ SDValue RISCVTargetLowering::PerformDAGCombine(SDNode *N,
           MGN->getMemOperand(), IndexType, MGN->getExtensionType());
 
     if (Index.getOpcode() == ISD::BUILD_VECTOR &&
-        MGN->getExtensionType() == ISD::NON_EXTLOAD) {
+        MGN->getExtensionType() == ISD::NON_EXTLOAD &&
+        isTypeLegal(VT)) {
       if (std::optional<VIDSequence> SimpleVID = isSimpleVIDSequence(Index);
           SimpleVID && SimpleVID->StepDenominator == 1) {
         const int64_t StepNumerator = SimpleVID->StepNumerator;

@lukel97
Copy link
Contributor

lukel97 commented Feb 8, 2024

Possible fix

diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 27037f4d5c5c..405d99f14d8f 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -15812,7 +15812,8 @@ SDValue RISCVTargetLowering::PerformDAGCombine(SDNode *N,
           MGN->getMemOperand(), IndexType, MGN->getExtensionType());
 
     if (Index.getOpcode() == ISD::BUILD_VECTOR &&
-        MGN->getExtensionType() == ISD::NON_EXTLOAD) {
+        MGN->getExtensionType() == ISD::NON_EXTLOAD &&
+        isTypeLegal(VT)) {
       if (std::optional<VIDSequence> SimpleVID = isSimpleVIDSequence(Index);
           SimpleVID && SimpleVID->StepDenominator == 1) {
         const int64_t StepNumerator = SimpleVID->StepNumerator;

This fixes it on my end. And the combine kicks in again once the MGATHER has been split, so we end up with 2 v16i64 vlse64s from the one v32i64 it was crashing on.

@topperc
Copy link
Collaborator

topperc commented Feb 8, 2024

@lukel97 can you make a PR?

lukel97 added a commit that referenced this issue Feb 8, 2024
#81107)

Otherwise we will crash since target intrinsics don't have their types
legalized. Let the mgather get legalized first, then do the combine on
the legal type.
Fixes #81088

Co-authored-by: Craig Topper <craig.topper@sifive.com>
llvmbot pushed a commit to llvmbot/llvm-project that referenced this issue Feb 13, 2024
llvm#81107)

Otherwise we will crash since target intrinsics don't have their types
legalized. Let the mgather get legalized first, then do the combine on
the legal type.
Fixes llvm#81088

Co-authored-by: Craig Topper <craig.topper@sifive.com>
(cherry picked from commit 06c89bd)
llvmbot pushed a commit to llvmbot/llvm-project that referenced this issue Feb 20, 2024
llvm#81107)

Otherwise we will crash since target intrinsics don't have their types
legalized. Let the mgather get legalized first, then do the combine on
the legal type.
Fixes llvm#81088

Co-authored-by: Craig Topper <craig.topper@sifive.com>
(cherry picked from commit 06c89bd)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants