-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[RISCV] Support zilsd-4byte-align for i64 load/store in SelectionDAG. #169182
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
Conversation
I think we need to keep the SelectionDAG code for volatile load/store so we should support 4 byte alignment when possible.
|
@llvm/pr-subscribers-backend-risc-v Author: Craig Topper (topperc) ChangesI think we need to keep the SelectionDAG code for volatile load/store so we should support 4 byte alignment when possible. Full diff: https://github.com/llvm/llvm-project/pull/169182.diff 2 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVSubtarget.h b/llvm/lib/Target/RISCV/RISCVSubtarget.h
index 29df53c6c9893..b659bb96f2f11 100644
--- a/llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ b/llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -237,6 +237,13 @@ class RISCVSubtarget : public RISCVGenSubtargetInfo {
return 0;
}
+
+ Align getZilsdAlign() const {
+ return Align(enableUnalignedScalarMem() ? 1
+ : allowZilsd4ByteAlign() ? 4
+ : 8);
+ }
+
unsigned getELen() const {
assert(hasVInstructions() && "Expected V extension");
return hasVInstructionsI64() ? 64 : 32;
diff --git a/llvm/lib/Target/RISCV/RISCVZilsdOptimizer.cpp b/llvm/lib/Target/RISCV/RISCVZilsdOptimizer.cpp
index 99e83fbb05a73..3b47903c351bf 100644
--- a/llvm/lib/Target/RISCV/RISCVZilsdOptimizer.cpp
+++ b/llvm/lib/Target/RISCV/RISCVZilsdOptimizer.cpp
@@ -146,9 +146,7 @@ bool RISCVPreAllocZilsdOpt::runOnMachineFunction(MachineFunction &MF) {
// Check alignment: default is 8-byte, but allow 4-byte with tune feature
// If unaligned scalar memory is enabled, allow any alignment
- RequiredAlign = STI->enableUnalignedScalarMem() ? Align(1)
- : STI->allowZilsd4ByteAlign() ? Align(4)
- : Align(8);
+ RequiredAlign = STI->getZilsdAlign();
bool Modified = false;
for (auto &MBB : MF) {
Modified |= rescheduleLoadStoreInstrs(&MBB);
|
|
Did you leave out the SDag changes? This is just a Subtarget change right now. |
Weird. I wrote a SelectionDAG change and a test. I don't know where they went. |
lenary
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
4vtomat
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM~
|
do we also want to handle non-volatile case in selection dag? |
I think we need to keep the SelectionDAG code for volatile load/store so we should support 4 byte alignment when possible.