[RISCV] Qualify all XCV predicates with !is64Bit. #101074
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The tablegen patterns all have isRV32. I did not check if any of them could naively support RV64. Fixes llvm#101067 and probably other bugs like it we haven't found yet. Not sure how value the one test case is given that it would only be testing one or two of the places I touched.
|
@llvm/pr-subscribers-backend-risc-v Author: Craig Topper (topperc) ChangesThe tablegen patterns all have isRV32. I did not check if any of them could naively support RV64. Fixes #101067 and probably other bugs like it we haven't found yet. Not sure how valuable adding a test case is given that it would only be testing one or two of the places I touched. Full diff: https://github.com/llvm/llvm-project/pull/101074.diff 3 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
index 01b2bc08d3ba0..d08430ca5ecfc 100644
--- a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
@@ -1540,7 +1540,7 @@ void RISCVDAGToDAGISel::Select(SDNode *Node) {
if (tryIndexedLoad(Node))
return;
- if (Subtarget->hasVendorXCVmem()) {
+ if (Subtarget->hasVendorXCVmem() && !Subtarget->is64Bit()) {
// We match post-incrementing load here
LoadSDNode *Load = cast<LoadSDNode>(Node);
if (Load->getAddressingMode() != ISD::POST_INC)
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 9ce669a3122f5..42873d4511a10 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -250,14 +250,14 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
if (RV64LegalI32 && Subtarget.is64Bit())
setOperationAction(ISD::SELECT_CC, MVT::i32, Expand);
- if (!Subtarget.hasVendorXCValu())
- setCondCodeAction(ISD::SETLE, XLenVT, Expand);
setCondCodeAction(ISD::SETGT, XLenVT, Custom);
setCondCodeAction(ISD::SETGE, XLenVT, Expand);
- if (!Subtarget.hasVendorXCValu())
- setCondCodeAction(ISD::SETULE, XLenVT, Expand);
setCondCodeAction(ISD::SETUGT, XLenVT, Custom);
setCondCodeAction(ISD::SETUGE, XLenVT, Expand);
+ if (!(Subtarget.hasVendorXCValu() && !Subtarget.is64Bit())) {
+ setCondCodeAction(ISD::SETULE, XLenVT, Expand);
+ setCondCodeAction(ISD::SETLE, XLenVT, Expand);
+ }
if (RV64LegalI32 && Subtarget.is64Bit())
setOperationAction(ISD::SETCC, MVT::i32, Promote);
@@ -343,7 +343,7 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
if (Subtarget.is64Bit())
setOperationAction({ISD::ROTL, ISD::ROTR}, MVT::i32, Custom);
setOperationAction({ISD::ROTL, ISD::ROTR}, XLenVT, Custom);
- } else if (Subtarget.hasVendorXCVbitmanip()) {
+ } else if (Subtarget.hasVendorXCVbitmanip() && !Subtarget.is64Bit()) {
setOperationAction(ISD::ROTL, XLenVT, Expand);
} else {
setOperationAction({ISD::ROTL, ISD::ROTR}, XLenVT, Expand);
@@ -366,7 +366,7 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
: Expand);
- if (Subtarget.hasVendorXCVbitmanip()) {
+ if (Subtarget.hasVendorXCVbitmanip() && !Subtarget.is64Bit()) {
setOperationAction(ISD::BITREVERSE, XLenVT, Legal);
} else {
// Zbkb can use rev8+brev8 to implement bitreverse.
@@ -387,14 +387,14 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
else
setOperationAction({ISD::CTTZ, ISD::CTTZ_ZERO_UNDEF}, MVT::i32, Custom);
}
- } else if (!Subtarget.hasVendorXCVbitmanip()) {
+ } else if (!(Subtarget.hasVendorXCVbitmanip() && !Subtarget.is64Bit())) {
setOperationAction({ISD::CTTZ, ISD::CTPOP}, XLenVT, Expand);
if (RV64LegalI32 && Subtarget.is64Bit())
setOperationAction({ISD::CTTZ, ISD::CTPOP}, MVT::i32, Expand);
}
if (Subtarget.hasStdExtZbb() || Subtarget.hasVendorXTHeadBb() ||
- Subtarget.hasVendorXCVbitmanip()) {
+ (Subtarget.hasVendorXCVbitmanip() && !Subtarget.is64Bit())) {
// We need the custom lowering to make sure that the resulting sequence
// for the 32bit case is efficient on 64bit targets.
if (Subtarget.is64Bit()) {
@@ -1439,7 +1439,7 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
}
}
- if (Subtarget.hasVendorXCVmem()) {
+ if (Subtarget.hasVendorXCVmem() && !Subtarget.is64Bit()) {
setIndexedLoadAction(ISD::POST_INC, MVT::i8, Legal);
setIndexedLoadAction(ISD::POST_INC, MVT::i16, Legal);
setIndexedLoadAction(ISD::POST_INC, MVT::i32, Legal);
@@ -1449,7 +1449,7 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
setIndexedStoreAction(ISD::POST_INC, MVT::i32, Legal);
}
- if (Subtarget.hasVendorXCValu()) {
+ if (Subtarget.hasVendorXCValu() && !Subtarget.is64Bit()) {
setOperationAction(ISD::ABS, XLenVT, Legal);
setOperationAction(ISD::SMIN, XLenVT, Legal);
setOperationAction(ISD::UMIN, XLenVT, Legal);
@@ -1928,12 +1928,13 @@ bool RISCVTargetLowering::signExtendConstant(const ConstantInt *CI) const {
}
bool RISCVTargetLowering::isCheapToSpeculateCttz(Type *Ty) const {
- return Subtarget.hasStdExtZbb() || Subtarget.hasVendorXCVbitmanip();
+ return Subtarget.hasStdExtZbb() ||
+ (Subtarget.hasVendorXCVbitmanip() && !Subtarget.is64Bit());
}
bool RISCVTargetLowering::isCheapToSpeculateCtlz(Type *Ty) const {
return Subtarget.hasStdExtZbb() || Subtarget.hasVendorXTHeadBb() ||
- Subtarget.hasVendorXCVbitmanip();
+ (Subtarget.hasVendorXCVbitmanip() && !Subtarget.is64Bit());
}
bool RISCVTargetLowering::isMaskAndCmp0FoldingBeneficial(
@@ -21082,7 +21083,7 @@ bool RISCVTargetLowering::getPostIndexedAddressParts(SDNode *N, SDNode *Op,
SDValue &Offset,
ISD::MemIndexedMode &AM,
SelectionDAG &DAG) const {
- if (Subtarget.hasVendorXCVmem()) {
+ if (Subtarget.hasVendorXCVmem() && !Subtarget.is64Bit()) {
if (Op->getOpcode() != ISD::ADD)
return false;
diff --git a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
index a61a9f10be86c..4cd904c039a98 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
@@ -280,7 +280,7 @@ bool RISCVTTIImpl::hasActiveVectorLength(unsigned, Type *DataTy, Align) const {
TargetTransformInfo::PopcntSupportKind
RISCVTTIImpl::getPopcntSupport(unsigned TyWidth) {
assert(isPowerOf2_32(TyWidth) && "Ty width must be power of 2");
- return ST->hasStdExtZbb() || ST->hasVendorXCVbitmanip()
+ return ST->hasStdExtZbb() || (ST->hasVendorXCVbitmanip() && !ST->is64Bit())
? TTI::PSK_FastHardware
: TTI::PSK_Software;
}
|
You can test this locally with the following command:git-clang-format --diff 7b99b1d4f733ed5e1b206f0b392b0864e7a0d468 33eba10548577b347589a1875f81a3f7e3126de1 --extensions cpp -- llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp llvm/lib/Target/RISCV/RISCVISelLowering.cpp llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cppView the diff from clang-format here.diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 42873d4511..23e2ed2d4d 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -365,7 +365,6 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
? Promote
: Expand);
-
if (Subtarget.hasVendorXCVbitmanip() && !Subtarget.is64Bit()) {
setOperationAction(ISD::BITREVERSE, XLenVT, Legal);
} else {
|
wangpc-pp
reviewed
Jul 30, 2024
There was a problem hiding this comment.
LTGM but I don't know if these extensions will be supported in 64 bits core like https://github.com/openhwgroup/cvw.
Hopefully someone working on XCV can help. CC @realqhc @melonedo @PaoloS02 |
|
We have verified on the meeting today that there is no plan to support XCV extensions for 64 bits core for now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The tablegen patterns all have isRV32. I did not check if any of them could naively support RV64.
Fixes #101067 and probably other bugs like it we haven't found yet.
Not sure how valuable adding a test case is given that it would only be testing one or two of the places I touched.