[patch] MicroBlaze Backend

Project Infrastructure LLVM Dev List Archives
#1

I have been working on a LLVM backend for the MicroBlaze soft-processor:

Attached is the initial MicroBlaze patch. It does the following:
1. Adds mblaze as a target in configure and configure.ac
2. Adds mblaze specific intrinsics in include/llvm/IntrinsicsMBlaze.td and include/llvm/Intrinsics.td
3. Adds mblaze triple support in include/llvm/ADT/Triple.h and lib/Support/Triple.cpp
4. Adds mblaze backend support in lib/Target/MBlaze
5. Adds mblaze test cases in test/CodeGen/MBlaze

Currently the MicroBlaze backend generates MicroBlaze assembly which compiles successfully using the MicroBlaze assembler. Many small to medium sized test cases seem to compile and work but extensive testing has not yet been done.

I have not supplied any major patches to the LLVM development team before so if the patch needs to be reworked then let me know and I will take care of it.

2010-01-29-mblaze.patch.gz (44.9 KB)

#2

Hello, Wesley

Attached is the initial MicroBlaze patch. It does the following:
1. Adds mblaze as a target in configure and configure.ac
2. Adds mblaze specific intrinsics in include/llvm/IntrinsicsMBlaze.td and include/llvm/Intrinsics.td
3. Adds mblaze triple support in include/llvm/ADT/Triple.h and lib/Support/Triple.cpp
4. Adds mblaze backend support in lib/Target/MBlaze
5. Adds mblaze test cases in test/CodeGen/MBlaze

Thanks, I'll take care about review of this patch.

#3

I have been working on a LLVM backend for the MicroBlaze soft-processor:
http://www.xilinx.com/tools/microblaze.htm
http://en.wikipedia.org/wiki/MicroBlaze

Very Cool!

Attached is the initial MicroBlaze patch. It does the following:
1. Adds mblaze as a target in configure and configure.ac
2. Adds mblaze specific intrinsics in include/llvm/IntrinsicsMBlaze.td and include/llvm/Intrinsics.td
3. Adds mblaze triple support in include/llvm/ADT/Triple.h and lib/Support/Triple.cpp
4. Adds mblaze backend support in lib/Target/MBlaze
5. Adds mblaze test cases in test/CodeGen/MBlaze

Currently the MicroBlaze backend generates MicroBlaze assembly which compiles successfully using the MicroBlaze assembler. Many small to medium sized test cases seem to compile and work but extensive testing has not yet been done.

Your patch looks very clean. Some comments:

- You don't need the -mb-has-* options in MBlazeSubtarget.cpp. You can pass -mattr=+mul,-div to llc and ParseSubtargetFeatures will take care of the rest.

- Please try to move your intrinsics into the target subdirectory. So far only Blackfin has done that, but we would like to keep everything target-specific in lib/Target/MBlaze.

- Please use lower-case register names in assembly if the assembler allows it.

- I think you have some literal tabs in your instruction descriptions.

- 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.

The MicroBlaze is highly configurable and fairly simple. It would be a great addition to LLVM, also for educational purposes.

/jakob

#4

Your patch looks very clean. Some comments:

Heh, Jakob was faster :slight_smile:

- 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 :slight_smile:

- 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 :slight_smile:

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!

#5

Your patch looks very clean. Some comments:

Heh, Jakob was faster :slight_smile:

I have taken care of everything Jakob mentioned except the extra test cases. I will get to these as soon as I can.

- 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 :slight_smile:

I cherry-picked a lot of code from the Mips, Sparc, and PowerPC backends. That's probably why there are multiple conventions in use. I will try to clean this up as I continue to make changes.

- 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 :slight_smile:

I already have a microblaze description in clang but I have not tested it as much so I have not submitted a patch for it yet.

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.

I was unsure as to whether I needed this or not. I have been taking the approach of removing things conservatively as I continue to improve the backend. I will take a look at removing this for the next patch.

+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

Done.

+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.

These have not been completed yet. Some of the earlier processors don't support all of the instructions but I have not gotten around to enumerating all of the features that each processor supports. This will probably be done for the next patch.

+ 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?

The microblaze has only 32-bit registers. Should this be "E-p:32:32:32-i8:8:32-i16:16:32-n32" instead?

+static cl::opt<bool> HasFPUOpt(
+ "mb-has-fpu",

As Jakob already said - remove these, you should use subtarget
features (-mattr=+foo) instead.

Done.

As I understand, some features are not hooked yet, like

+def FeaturePipe3 : SubtargetFeature<"pipe3", "HasPipe3", "true",
+ "Implements 3-stage pipeline.">;

The microblaze can be configured for either a 3-stage pipeline or a 5-stage pipeline at system synthesis time. The three stage pipeline has lower area but also lower performance. This should eventually have an impact on the microblaze functional unit scheduling, but I have not gotten there yet.

#6

The MicroBlaze can do 8- and 16-bit loads and stores, so your first version is right.

You may want to relax the alignment for types larger than 32 bits. i64, f64, v64, and v128 probably only need 32-bit alignment.

/jakob

#7

Hello, Wesley

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.

I was unsure as to whether I needed this or not. I have been taking the approach of removing things conservatively as I continue to improve the backend. I will take a look at removing this for the next patch.

Well, the code to trigger this is quite simple:

define void @t() nounwind {
A:
        br label %entry

entry:
        %m1 = alloca i32, align 4
        %m2 = alloca [7 x i8], align 16
        call void @s( i32* %m1, [7 x i8]* %m2 )
        ret void
}

declare void @s(i32*, [7 x i8]*)