-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[RISCV] Fix bugs about getting register list of Zcmp push/pop. #66073
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
The original code encounters ICE when we need to spill callee saves but none of them is GPR.
@llvm/pr-subscribers-backend-risc-v ChangesThe original code encounters ICE when we need to spill callee saves but none of them is GPR.Full diff: https://github.com/llvm/llvm-project/pull/66073.diff 2 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp index add933250f8473d..898412ff446b075 100644 --- a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp +++ b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp @@ -1371,12 +1371,12 @@ bool RISCVFrameLowering::spillCalleeSavedRegisters( RISCVMachineFunctionInfo *RVFI = MF->getInfo(); if (RVFI->isPushable(*MF)) { Register MaxReg = getMaxPushPopReg(*MF, CSI); - unsigned PushedRegNum = - getPushPopEncoding(MaxReg) - llvm::RISCVZC::RLISTENCODE::RA + 1; - RVFI->setRVPushRegs(PushedRegNum); - RVFI->setRVPushStackSize(alignTo((STI.getXLen() / 8) * PushedRegNum, 16)); - if (MaxReg != RISCV::NoRegister) { + unsigned PushedRegNum = + getPushPopEncoding(MaxReg) - llvm::RISCVZC::RLISTENCODE::RA + 1; + RVFI->setRVPushRegs(PushedRegNum); + RVFI->setRVPushStackSize(alignTo((STI.getXLen() / 8) * PushedRegNum, 16)); + // Use encoded number to represent registers to spill. unsigned RegEnc = getPushPopEncoding(MaxReg); RVFI->setRVPushRlist(RegEnc); diff --git a/llvm/test/CodeGen/RISCV/zcmp-crash.ll b/llvm/test/CodeGen/RISCV/zcmp-crash.ll new file mode 100644 index 000000000000000..b22085f95a3d1b0 --- /dev/null +++ b/llvm/test/CodeGen/RISCV/zcmp-crash.ll @@ -0,0 +1,21 @@ +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2 +; RUN: llc -mtriple=riscv32 -target-abi ilp32f -mattr=+f,+zcmp -verify-machineinstrs < %s | FileCheck %s + +; Test the file could be compiled successfully. +define void @bar() { +; CHECK-LABEL: bar: +; CHECK: # %bb.0: # %entry +; CHECK-NEXT: addi sp, sp, -16 +; CHECK-NEXT: .cfi_def_cfa_offset 16 +; CHECK-NEXT: fsw fs0, 12(sp) # 4-byte Folded Spill +; CHECK-NEXT: .cfi_offset fs0, -4 +; CHECK-NEXT: #APP +; CHECK-NEXT: fmv.w.x fs0, zero +; CHECK-NEXT: #NO_APP +; CHECK-NEXT: flw fs0, 12(sp) # 4-byte Folded Reload +; CHECK-NEXT: addi sp, sp, 16 +; CHECK-NEXT: ret +entry: + tail call void asm sideeffect "fmv.w.x fs0, zero", "~{fs0}"() + ret void +} |
ICE is GCC terminology |
Thank you. I replace it with "internal` compiler error". |
I also adds another fix about register number of Zcmp push/pop. |
// Return encoded value for PUSH/POP instruction, representing | ||
// registers to store/load. |
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.
Comment need to update to reflect the update.
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.
Done.
; RV64IZCMP-WITH-FP-NEXT: addi sp, sp, 32 | ||
; RV64IZCMP-WITH-FP-NEXT: ret | ||
entry: | ||
tail call void asm sideeffect "li s4, 0", "~{s4}"() | ||
tail call void asm sideeffect "li s11, 0", "~{s11}"() |
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.
Add one more function instead of updating this?
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.
Done.
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.
I believe this is correct, would appreciate another quick review from someone who's been more active in the other zcmp patches though.
The pr does two things, one is to fix internal compiler error when we need to spill callee saves but none of them is GPR, another is to fix wrong register number for pushed registers are {ra, s0-s11}.