This patch adds more basic CSKY instructions including some privilege instructions. It involves a special operand representing register sequence like r0-r3. This operand is used at ldm/stm to load/store a sequence of register.
Diff Detail
Unit Tests
Event Timeline
The old 7 is now 9 and there's no 8? This is confusing.
Given that we have already merged the initial part, I think you can stop numbering the patches unless they're meant as a set, which then you start from one again (ex: 1/3, 2/3, 3/3).
Normally we would have gone all the way down small programs being generated (including ALU), but we started wrong, so let's not make it more confusing. :)
However, it's still important to know which patch depends on which, so make sure to still mark the parent/child dependencies correctly.
+1
Given that we have already merged the initial part, I think you can stop numbering the patches unless they're meant as a set, which then you start from one again (ex: 1/3, 2/3, 3/3).
Normally we would have gone all the way down small programs being generated (including ALU), but we started wrong, so let's not make it more confusing. :)
Agree. the now 9-th patch, which contains more fundamental instructions, might be better to come before this one
However, it's still important to know which patch depends on which, so make sure to still mark the parent/child dependencies correctly.
+1 I was not able to find this patch in the review stack either
Because there is other patch between 7 and 9, which I will make a revision later. As I plan to delay the patch of number 9 (codegen) until all basic instructions have been upstreamed, I renumber the 9 to next and next as need. Or we just abandon the prefix pattern like [CSKY 7/n]?
I thought as much, but it's confusing because it's not public. :)
As I plan to delay the patch of number 9 (codegen) until all basic instructions have been upstreamed, I renumber the 9 to next and next as need.
The number series is an information for the reviewers to know there are more dependencies up front, on a closed series.
So, either you upload all patches at once and number them from 1 to N, or you don't number and just link them via parent/child.
Also, avoid posting patches that can't be merged on the current tree (like one after a private one). It makes review a lot harder because we don't know your internal order, only what's public.
Or we just abandon the prefix pattern like [CSKY 7/n]?
I think we better drop the numbers for now. Just make sure you have the right parent/child relationship and post patches that can be merged directly (with all the clear public dependencies merged before).
Thanks!
--renato
llvm/lib/Target/CSKY/CSKYInstrInfo.td | ||
---|---|---|
464 | It's reserved for predicts at next patch. |
llvm/lib/Target/CSKY/AsmParser/CSKYAsmParser.cpp | ||
---|---|---|
208 | As clang tidy suggested, I think local variables should be capitalized | |
210 | Is this debug message? If that's the case please use LLVM_DEBUG macro and errs() / dbgs() stream | |
230 | Can we use std::make_pair here? The current syntax is a little too verbose | |
672 | Can we use explicit type here? | |
687 | ditto | |
llvm/lib/Target/CSKY/MCTargetDesc/CSKYInstPrinter.cpp | ||
101 | Can we use explicit type for Ry and Rz? |
clang-tidy: error: 'MCTargetDesc/CSKYMCExpr.h' file not found [clang-diagnostic-error]
not useful