[LLVMdev] [patch] MicroBlaze Backend

[Anton Korobeynikov]

> Your patch looks very clean. Some comments:
Heh, Jakob was faster :)

> - I think you have some literal tabs in your instruction descriptions.
The tabs can be seen in some other places as well. Also, there is a
"mix" of coding conventions in the files. It will be really nice to
use only one :)

> - Your tests are nice, but you could use some more of them. I would recommend tests for tricky calling convention stuff, some loops, switches and branches. Also indirect calls and indirect branches. Have you tries running Codegen/generic through your back end? That should give you some more test cases.
Some "feature" tests can be found inside other directories as well,
e.g. msp430 / systemz.

Are you planning to add microblaze description to clang? C compiler
can speed up testing alot :)

More comments:
>+SDValue MBlazeTargetLowering::
>+LowerDYNAMIC_STACKALLOC(SDValue Op, SelectionDAG &DAG) {
Do you really need this? Expanding dynamic allocas normally ends with
stack register adjustment, you don't need anything special here.

>+bool MBlazeTargetLowering::
>+SelectAddrRegReg(SDValue N, SDValue &Base, SDValue &Index, SelectionDAG &DAG) {
Move this to ISelDAGToDAG file

>+bool MBlazeTargetLowering::
>+SelectAddrRegImm(SDValue N, SDValue &Disp, SDValue &Base, SelectionDAG &DAG) {
Likewise

>+def : Proc<"v400", []>;
>+def : Proc<"v500", []>;
>+def : Proc<"v600", []>;
>+def : Proc<"v700", []>;
>+def : Proc<"v710", []>;
What's the difference between them then? isVN00() predicates are not used.

>+  DataLayout(std::string("E-p:32:32-i8:8:8-i16:16:16-n32")),
I'm confused. What are the native types for this arch?

>+static cl::opt<bool> HasFPUOpt(
>+    "mb-has-fpu",
As Jakob already said - remove these, you should use subtarget
features (-mattr=+foo) instead.

As I understand, some features are not hooked yet, like
>+def FeaturePipe3       : SubtargetFeature<"pipe3", "HasPipe3", "true",
>+                                "Implements 3-stage pipeline.">;

?

Otherwise patch looks good. Thanks!

-- 
With best regards, Anton Korobeynikov
Faculty of Mathematics and Mechanics, Saint Petersburg State University