Skip to content
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

[RISCV] Update DecoderMethod and MCOperandPredicate of spimm. #76061

Merged
merged 1 commit into from
Dec 27, 2023

Conversation

yetingk
Copy link
Contributor

@yetingk yetingk commented Dec 20, 2023

The spimm operand is an immediate that only its 4-5th bits could be setted.

@yetingk yetingk changed the title [RISCV] Update DecoderMethod and MCOperandPredicate spimm. [RISCV] Update DecoderMethod and MCOperandPredicate of spimm. Dec 20, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 20, 2023

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

Author: Yeting Kuo (yetingk)

Changes

The spimm operand is an immediate whose only 4-5th bits could be setted.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp (+2-2)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoZc.td (+1-1)
diff --git a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
index 53e2b6b4d94ea0..5c4dbb2249a116 100644
--- a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
+++ b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
@@ -462,10 +462,10 @@ static DecodeStatus decodeRegReg(MCInst &Inst, uint32_t Insn, uint64_t Address,
   return MCDisassembler::Success;
 }
 
-// spimm is based on rlist now.
 static DecodeStatus decodeZcmpSpimm(MCInst &Inst, unsigned Imm,
                                     uint64_t Address, const void *Decoder) {
-  // TODO: check if spimm matches rlist
+  if (!isShiftedUInt<2, 4>(Imm))
+    return MCDisassembler::Fail;
   Inst.addOperand(MCOperand::createImm(Imm));
   return MCDisassembler::Success;
 }
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td b/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
index a78f3624446871..2b3f08bc4a1a4f 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
@@ -70,7 +70,7 @@ def spimm : Operand<OtherVT> {
     int64_t Imm;
     if (!MCOp.evaluateAsConstantImm(Imm))
       return false;
-    return isShiftedUInt<5, 4>(Imm);
+    return isShiftedUInt<2, 4>(Imm);
   }];
 }
 

@yetingk
Copy link
Contributor Author

yetingk commented Dec 26, 2023

Ping.

@topperc
Copy link
Collaborator

topperc commented Dec 26, 2023

Is this testable?

static DecodeStatus decodeZcmpSpimm(MCInst &Inst, unsigned Imm,
uint64_t Address, const void *Decoder) {
// TODO: check if spimm matches rlist
if (!isShiftedUInt<2, 4>(Imm))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't this check can ever fail. The caller looks like

    tmp = fieldFromInstruction(insn, 2, 2) << 4;                                 
    if (!Check(S, decodeZcmpSpimm(MI, tmp, Address, Decoder))) { return MCDisassembler::Fail; }

So its guaranteed to be a isShiftedUInt<2, 4>

Copy link
Contributor Author

@yetingk yetingk Dec 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this check. Sorry I didn't survey enough code about this.

The spimm operand is an immediate whose only 4-5th bit could be setted
and not based on rlist operand.
Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yetingk yetingk merged commit 256bf56 into llvm:main Dec 27, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants