[PowerPC] Remove non-existent operand of CP_COPY instruction #153867
Conversation
|
@llvm/pr-subscribers-backend-powerpc Author: Sergei Barannikov (s-barannikov) ChangesFull diff: https://github.com/llvm/llvm-project/pull/153867.diff 1 Files Affected:
diff --git a/llvm/lib/Target/PowerPC/PPCInstr64Bit.td b/llvm/lib/Target/PowerPC/PPCInstr64Bit.td
index fd2084398c857..d0d77c2e28320 100644
--- a/llvm/lib/Target/PowerPC/PPCInstr64Bit.td
+++ b/llvm/lib/Target/PowerPC/PPCInstr64Bit.td
@@ -2000,7 +2000,7 @@ def : Pat<(int_ppc_darnraw), (DARN 2)>;
class X_RA5_RB5<bits<6> opcode, bits<10> xo, string opc, RegisterOperand ty,
InstrItinClass itin, list<dag> pattern>
- : X_L1_RS5_RS5<opcode, xo, (outs), (ins ty:$RA, ty:$RB, u1imm:$L),
+ : X_L1_RS5_RS5<opcode, xo, (outs), (ins ty:$RA, ty:$RB),
!strconcat(opc, " $RA, $RB"), itin, pattern>{
let L = 1;
}
|
|
ping? |
|
ping |
| @@ -2000,7 +2000,7 @@ def : Pat<(int_ppc_darnraw), (DARN 2)>; | |||
|
|
|||
| class X_RA5_RB5<bits<6> opcode, bits<10> xo, string opc, RegisterOperand ty, | |||
| InstrItinClass itin, list<dag> pattern> | |||
| : X_L1_RS5_RS5<opcode, xo, (outs), (ins ty:$RA, ty:$RB, u1imm:$L), | |||
| : X_L1_RS5_RS5<opcode, xo, (outs), (ins ty:$RA, ty:$RB), | |||
| !strconcat(opc, " $RA, $RB"), itin, pattern>{ | |||
| let L = 1; | |||
There was a problem hiding this comment.
This looks like it was meant to be used to generate the instr copy RA,RB,1.
In the class we have:
let Inst{10} = L;
Even thought we set this explicitly to 1 on line 2005, I am not sure how that works.
I would think the generated instr needs to be fixed up here instead?
!strconcat(opc, " $RA, $RB, $L"), itin, pattern>{
There was a problem hiding this comment.
From what I could google, copy has only two operands. The L bit is bit 10, which has fixed value of 1 in the encoding.
This is unlike paste instruction, which indeed has three operands:
I guess you could call it a copy&paste bug 😆
There was a problem hiding this comment.
I was referring to Power ISA™ Version 3.1 (2020). In version 3.0 B (2017) paste is two operand as well.
There was a problem hiding this comment.
You are right! I was looking at an older ISA.
| @@ -2000,7 +2000,7 @@ def : Pat<(int_ppc_darnraw), (DARN 2)>; | |||
|
|
|||
| class X_RA5_RB5<bits<6> opcode, bits<10> xo, string opc, RegisterOperand ty, | |||
| InstrItinClass itin, list<dag> pattern> | |||
| : X_L1_RS5_RS5<opcode, xo, (outs), (ins ty:$RA, ty:$RB, u1imm:$L), | |||
| : X_L1_RS5_RS5<opcode, xo, (outs), (ins ty:$RA, ty:$RB), | |||
| !strconcat(opc, " $RA, $RB"), itin, pattern>{ | |||
| let L = 1; | |||
There was a problem hiding this comment.
You are right! I was looking at an older ISA.
|
Thank you for the review |
The operand is not encoded, decoded or printed and would break MCInst verification if we had one.
Extracted from #156358, where the extra operand causes DecoderEmitter to emit an error about an operand with a missing encoding.