craig.topper added inline comments.
================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:326
+ unsigned Val;
+ bool IsRV64;
+ };
----------------
Is this IsRV64 field used?
================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:331
+ unsigned Val;
+ bool IsRV64;
+ };
----------------
Is this IsRV64 field used?
================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:1028
+ // be called after checking isFRMArg.
+ RISCVFPRndMode::RoundingMode getRoundingMode() const {
+ // isFRMArg has validated the operand, meaning this cast is safe.
----------------
`getRoundingMode` shouldn't be in this patch
================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2120
+ StringRef EndName = getLexer().getTok().getIdentifier();
+ // FixMe: the register mapping and checks of EABI is wrong
+ if (matchRegisterNameHelper(/*IsEABI*/ false, RegEnd, EndName))
----------------
FixMe -> FIXME
================
Comment at: llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp:451
+ const void *Decoder) {
+ // Sign-extend the number in the bottom N bits of Imm
+ if (Imm <= 3)
----------------
This comment looks copy/pasted from somewhere else?
================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp:224
+ else if (SlistEncode > 5 && SlistEncode <= 14)
+ OS << "-s" + std::to_string(SlistEncode - 5);
+ }
----------------
Does this work?
```
OS << "-s" << (SlistEncode - 5);
```
================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h:492
+ RA_S0_S11,
+ RA_S0_S10, // This is for error checking.
+};
----------------
Should use this define instead writing 16 in several places? Or maybe rename
this so it's obviously invalid?
================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp:200
+ if (Spimm < Base || Spimm > Base + 48)
+ llvm_unreachable("Incorrect spimm");
+ if (Opcode == RISCV::CM_PUSH) {
----------------
Combine the `if` into an `assert`?
================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp:97
+private:
+ FeatureBitset computeAvailableFeatures(const FeatureBitset &FB) const;
+ void
----------------
What are these functions?
================
Comment at: llvm/lib/Target/RISCV/RISCVFeatures.td:335
+def HasStdExtZcmp : Predicate<"Subtarget->hasStdExtZcmp() &&
!Subtarget->hasStdExtC()">,
+ AssemblerPredicate<(all_of FeatureExtZcmp, (not
FeatureStdExtC)),
+ "'Zcmp' (sequenced instuctions for code-size
reduction.)">;
----------------
Line `AssemblerPredicate` up with the `"Subtarget` on the previous line.
================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZc.td:70
+ return false;
+ return isShiftedUInt<5, 4>(Imm);
+ }];
----------------
Extra space before `isShiftedUInt`
================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZc.td:85
+let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
+class RVZcArith_rr<bits<6> funct6,bits<2> funct2, bits<2> opcode, string
opcodestr>
----------------
These lines were deleted when the Zcb patch was merged.
================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZc.td:260
(C_ZEXT_H GPRC:$rs1, GPRC:$rs1)>;
-} // Predicates = [HasStdExtZcb, HasStdExtZbb]
+} // Predicates = [, HasStdExtZbb]
----------------
What happened here?
================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.td:177
+// Saved Registers from s0 to s7, for C.MVA01S07 instruction in Zc extension
+def SR07 : RegisterClass<"RISCV", [XLenVT], 32, (add
----------------
Zc -> Zcmp?
================
Comment at: llvm/lib/Target/RISCV/RISCVSchedRocket.td:21
let UnsupportedFeatures = [HasStdExtZbkb, HasStdExtZbkc, HasStdExtZbkx,
- HasStdExtZknd, HasStdExtZkne, HasStdExtZknh,
- HasStdExtZksed, HasStdExtZksh, HasStdExtZkr,
- HasVInstructions, HasVInstructionsI64];
+ HasStdExtZcb, HasStdExtZknd, HasStdExtZkne,
+ HasStdExtZknh, HasStdExtZksed, HasStdExtZksh,
----------------
While this is probably correct, since it says Zcb and not Zcmp, it doesn't
belong in this patch.
================
Comment at: llvm/lib/Target/RISCV/RISCVSchedSiFive7.td:18
let CompleteModel = 0;
let UnsupportedFeatures = [HasStdExtZbkb, HasStdExtZbkc, HasStdExtZbkx,
+ HasStdExtZcb, HasStdExtZknd, HasStdExtZkne,
----------------
Same
================
Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:83
initializeRISCVInsertVSETVLIPass(*PR);
- initializeRISCVDAGToDAGISelPass(*PR);
}
----------------
????
================
Comment at: llvm/test/MC/RISCV/attribute-arch.s:170
-.attribute arch, "rv32izihintntl0p2"
-# CHECK: attribute 5, "rv32i2p0_zihintntl0p2"
----------------
??????
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132819/new/
https://reviews.llvm.org/D132819
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
