Details
Details
- Reviewers
asb
Diff Detail
Diff Detail
Event Timeline
Comment Actions
Hi Ahmed. Many thanks for the contribution, and sorry for the slight delay in reviewing.
There are a couple of things you can do to make it easier to review:
- Follow the advice on requesting a review via phabricator and generate a patch with full context. This makes it much easier to review from the web interface
- Make use of clang-format to ensure indentation etc matches the LLVM coding style. See here. Though note clang-format will try to reformat the table in RISCVAsmBackend.cpp - revert that change. It might actually be worth adding comments to disable clang-format around that table.
Some minor formatting issues, but otherwise this is looking good to me once those are addressed. Thanks!
lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp | ||
---|---|---|
58–61 | Indentation is incorrect (clang-format is your friend!) | |
lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp | ||
170–174 | Duplicated break, and clang-format will suggest the slightly nicer: + FixupKind = MIFrm == RISCVII::InstFormatI + ? RISCV::fixup_riscv_pcrel_lo12_i + : RISCV::fixup_riscv_pcrel_lo12_s; | |
lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp | ||
70–72 | Over-indented |
Comment Actions
Thanks, I was looking for the code review docs but apparently I was looking in the wrong place. :)
Indentation is incorrect (clang-format is your friend!)