fpallares added inline comments.
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:542
defm VMSGT_V : VALU_IV_X_I<"vmsgt", 0b01>;
+}
Minor nit: Add comment here (for the other `let RVVConstraint = NoConstraint`
blocks below as well).
Repository:
rG
fpallares accepted this revision.
fpallares added a comment.
This revision is now accepted and ready to land.
Aside from the minor nit below this patch LGTM. Thanks a lot for all the
changes @HsiangKai.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/
fpallares added a comment.
Thanks for the update @HsiangKai.
I've noticed that we aren't handling the exceptions that state that the `V0`
constraint shouldn't be enforced for instructions that generate masks or for
reductions.
For instance the following (valid) instructions are rejected:
vm
fpallares added a comment.
Apologies we didn't identify this earlier but with the change of the mask
register layout (`MLEN=1`) the overlap constraints involving the mask register
were modified:
//**RVV-0.8, Section 5.3. Vector Masking:**//
> The destination vector register group for a masked
fpallares added inline comments.
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:99
// load vd, (rs1), vm
class VUnitStrideLoad
I believe that with the changes introduced in the encoding of the loads and
stores we can do without the `mop` parameter in mos
fpallares added inline comments.
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2386
+CheckReg = Inst.getOperand(3).getReg();
}
+if (DestReg == CheckReg)
fpallares wrote:
> With the suggestion above, this could be further simplifi
fpallares added inline comments.
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2367
+Opcode == RISCV::VADC_VIM || Opcode == RISCV::VSBC_VVM ||
+Opcode == RISCV::VSBC_VXM) {
+ if (DestReg == RISCV::V0)
I think we might not nee
fpallares added inline comments.
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2362
+ }
+ if (TargetFlags & RISCV::VMConstraint) {
+// vadc, vsbc are special cases.
Given that this constraint has no effect when `DestReg != RISCV::V0`, we co
fpallares added a comment.
Hi @HsiangKai, thanks for the patch. So far everything looks good aside from a
couple of minor nits.
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:1638
+
+ if (getLexer().getKind() == AsmToken::EndOfStatement) {
+Operands.push_b
fpallares added a comment.
I'm not a reviewer but the patch LGTM, thanks for all the changes @HsiangKai.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69987/new/
https://reviews.llvm.org/D69987
___
cfe
fpallares added a comment.
I've added a couple more inline comments.
I've also noticed that we aren't emitting the aliases for unmasked
instructions, for instance:
(input → output)
vnot.v v8, v4, v0.t → vnot.v v8, v4, v0.t
vnot.v v8, v4 → vxor.vi v8, v4, -1
AFAICT RISC-V doesn'
fpallares added inline comments.
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2283
+ unsigned Src2Reg = Inst.getOperand(1).getReg();
+ if (DestReg == Src2Reg)
+return Error(Loc, "The destination vector register group cannot
overlap"
--
fpallares added a comment.
Hi Kai, I've added an inline comment regarding constraint validation:
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2283
+ unsigned Src2Reg = Inst.getOperand(1).getReg();
+ if (DestReg == Src2Reg)
+return Error(Loc,
fpallares added a comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69987/new/
https://reviews.llvm.org/D69987
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/
14 matches
Mail list logo