Complete the basic integer instruction set and add related predictor in CSKY.td. And it includes the instruction definition and asm parser support. Following patches would be codegen pattern.
Details
Diff Detail
Event Timeline
I have a few comments, but otherwise, it's looking good. Thanks!
llvm/lib/Target/CSKY/AsmParser/CSKYAsmParser.cpp | ||
---|---|---|
98 | extra comment? | |
124 | From To? To match the ones below? | |
222 | This is getting too long, why can't you just use the template version directly? | |
283 | Do you really need this templated version? | |
399 | Shouldn't this be an error? Or an assert? | |
llvm/lib/Target/CSKY/CSKYInstrInfo.td | ||
312 | Sorry, I'm missing the context... Why isn't this a GPR? Is i32 a 32-bit view of the register (like AArch64)? If so, then i32 is confusing because of the IR types. Perhaps GPR32? |
llvm/lib/Target/CSKY/AsmParser/CSKYAsmParser.cpp | ||
---|---|---|
222 | Because the interface is defined in generated inc file like isUImm1(), isUImm2()... | |
283 | For now only used in isRegSeq(), but also used in float register sequence predictor isRegSeqF1() which is not upstreamed now. | |
399 | I think it's a print result, it may not make sense to assert to debug. | |
llvm/lib/Target/CSKY/CSKYInstrInfo.td | ||
312 | because GPR is also for DSP instruction so that it can also contain v4i8 and v2i16 type. |
llvm/lib/Target/CSKY/AsmParser/CSKYAsmParser.cpp | ||
---|---|---|
222 | I feel like I've asked this before... :( Sorry. Ignore me. | |
283 | Thought so, np. | |
399 | But isn't this going to also print into asm files? If so, then you could in theory print out broken asm files and only notice because the assembler crashes. | |
llvm/lib/Target/CSKY/CSKYInstrInfo.td | ||
312 | From CSKYRegisterInfo.td, GPR lists only R* registers. Do those alias with DPS registers? Other targets keep GPR as scalar registers and have other naming for registers that can alias FP/Vector registers. For example, AArch64 names GPR64 and GPR32 (and many other classes) to deal with those issues. You're better off naming the classes now and specialising later, than just use the bit-width. This makes it much simpler to read the table-gen files and also maintain them when they grow too large. |
llvm/lib/Target/CSKY/AsmParser/CSKYAsmParser.cpp | ||
---|---|---|
399 | It's not printed in asm file but in mir format in debug output. | |
llvm/lib/Target/CSKY/CSKYInstrInfo.td | ||
312 | If I understand correctly, you prefer to use another register class such as DSPR to represent v4i8 and v2i16 even if it also contains general register R0-R31 as GPR? |
llvm/lib/Target/CSKY/CSKYInstrInfo.td | ||
---|---|---|
312 | Yes, that's the idea. Sorry for going around in circles. :) |
extra comment?