Linux/ppc backend

Project Infrastructure LLVM Dev List Archives
1

Hi everyone,

I have almost completed the implementation of a linux/ppc backend in llvm. There were a few things to modify in
lib/Target/PowerPC with a lot of "if (!isDarwin)".

There are some places where I need help before saying the port is complete. I attached the diff file as a reference

1) In order to generate a creqv instruction before a vararg call, I created a new instruction in PPCInstrInfo.td: SETCR which
uses the new XForm_1_ext format. It does not use the XForm_1 format because I wanted to give only one register as operand.
I'm not sure if this is the correct way to do this, but it works.

2) Line 369 of PPCInstrInfo.td, we declare the non-callee saved registers. However, Linux and Darwin do not have the same set
of non-callee saved registers. I don't know how to make the if(isDarwin) test in here

3) R31, which replaces R1 as stack pointer when there is a dynamic allocation in a method, must be seen as a callee-saved register and must not be saved and restored like it is actually for Darwin. I don't know how to specify that, when there is a dynamic allocation, R31 must be added to the set of registers that are saved before entering the method and restored at the end.

If anyone's kind enough to help me out :slight_smile:

Cheers,
Nicolas

diff-powerpc-linux.patch (36.2 KB)

2

Nicolas,

Would you point me to the Linux/PPC ABI documents you are using so I can better judge what your restrictions are? These changes also have an effect on debugging and exception handling.

Cheers,

– Jim

3

I have almost completed the implementation of a linux/ppc backend in llvm.

Cool!

There were a few things to modify in
lib/Target/PowerPC with a lot of "if (!isDarwin)".

Some meta comments:

1. Please don't change PPC -> llvmPPC. I assume that you did this because
    PPC is a #define in some system header. Please just add a '#undef PPC'
    where appropriate to make this unneeded.

2. The X86 backend has the unfortunate habit of saying "if !darwin" "if
    !cygwin" etc. Most of the changes you've made are actually ABI related
    changes, not OS-specific changes. As such, I'd prefer it if you added
    two methods to PPCSubtarget: isMachoABI() and isELF_ABI() (or whatever
    is the right name of the ABI that linux uses) and use those instead.

3. Please split up the patch into independent chunks for easier review.
    This will make it much more likely that your pieces will be applied in
    a timely fashion :). This will also let you get pieces in before the
    whole thing is "done".

1) In order to generate a creqv instruction before a vararg call, I created a new instruction in PPCInstrInfo.td: SETCR which
uses the new XForm_1_ext format. It does not use the XForm_1 format because I wanted to give only one register as operand.
I'm not sure if this is the correct way to do this, but it works.

Yep, that's the right way to go.

2) Line 369 of PPCInstrInfo.td, we declare the non-callee saved registers. However, Linux and Darwin do not have the same set
of non-callee saved registers. I don't know how to make the if(isDarwin) test in here

Take a look at ARM/ARMRegisterInfo.td for an example of this.

3) R31, which replaces R1 as stack pointer when there is a dynamic allocation in a method, must be seen as a callee-saved register and must not be saved and restored like it is actually for Darwin. I don't know how to specify that, when there is a dynamic allocation, R31 must be added to the set of registers that are saved before entering the method and restored at the end.

Can you just mark it callee saved? If so, and if there is an instruction in the function that clobbers R31, it should automatically be saved and restored.

Thanks a lot for doing this, linux/ppc support is a major new feature!

-Chris

4

Hi Chris,

Chris Lattner wrote:

  
Some meta comments:

1. Please don't change PPC -> llvmPPC. I assume that you did this because
    PPC is a #define in some system header. Please just add a '#undef PPC'
    where appropriate to make this unneeded.
  

OK

2. The X86 backend has the unfortunate habit of saying "if !darwin" "if
    !cygwin" etc. Most of the changes you've made are actually ABI related
    changes, not OS-specific changes. As such, I'd prefer it if you added
    two methods to PPCSubtarget: isMachoABI() and isELF_ABI() (or whatever
    is the right name of the ABI that linux uses) and use those instead.
  

Well all of the changes are ABI related. In fact, could you give me an
example of what is os-specific and not abi-specific? I'm not sure.

3. Please split up the patch into independent chunks for easier review.
    This will make it much more likely that your pieces will be applied in
    a timely fashion :). This will also let you get pieces in before the
    whole thing is "done".
  

OK

2) Line 369 of PPCInstrInfo.td, we declare the non-callee saved registers. However, Linux and Darwin do not have the same set
of non-callee saved registers. I don't know how to make the if(isDarwin) test in here
    
Take a look at ARM/ARMRegisterInfo.td for an example of this.

Great, thx for the hint

3) R31, which replaces R1 as stack pointer when there is a dynamic allocation in a method, must be seen as a callee-saved register and must not be saved and restored like it is actually for Darwin. I don't know how to specify that, when there is a dynamic allocation, R31 must be added to the set of registers that are saved before entering the method and restored at the end.
    
Can you just mark it callee saved? If so, and if there is an instruction in the function that clobbers R31, it should automatically be saved and restored.

It is marked callee saved. Because when it is not needed as frame pointer
it is used like an ordinary register. But when it is used as frame pointer, the prologue
and epilogue change its value, but the algorithm in llvm that finds clobbered register
does not select it.

Thanks a lot for doing this, linux/ppc support is a major new feature!

No problem, that way I'll be able to work with llvm on my own box :slight_smile:

Thx,
Nicolas

5

2. The X86 backend has the unfortunate habit of saying "if !darwin" "if
    !cygwin" etc. Most of the changes you've made are actually ABI related
    changes, not OS-specific changes. As such, I'd prefer it if you added
    two methods to PPCSubtarget: isMachoABI() and isELF_ABI() (or whatever
    is the right name of the ABI that linux uses) and use those instead.

Well all of the changes are ABI related. In fact, could you give me an
example of what is os-specific and not abi-specific? I'm not sure.

Most of the things you are doing are ABI-related, and until we have multiple targets using the same ABI, it doesn't make sense to have os-specific aspects.

An example of an OS-specific aspect: cygwin and mingw on x86 use the same ABI, however, mingw refers to longjmp as "longjmp" and cygwin refers to it as "_longjmp".

3) R31, which replaces R1 as stack pointer when there is a dynamic allocation
in a method, must be seen as a callee-saved register and must not be saved
and restored like it is actually for Darwin. I don't know how to specify
that, when there is a dynamic allocation, R31 must be added to the set of
registers that are saved before entering the method and restored at the end.

Can you just mark it callee saved? If so, and if there is an instruction
in the function that clobbers R31, it should automatically be saved and
restored.

It is marked callee saved. Because when it is not needed as frame pointer it is used like an ordinary register. But when it is used as frame pointer, the prologue and epilogue change its value, but the algorithm in llvm that finds clobbered register does not select it.

Okay, I'm not sure. If you describe the constraints better, perhaps Jim or Evan will have an idea :slight_smile:

-Chris

6

Hi Jim,

I didn't use any documents, but intensively looked at gcc's output. I think this document:
http://refspecs.freestandards.org/elf/elfspec_ppc.pdf

is the last specification of the ABI.

Cheers,
Nicolas

Jim Laskey wrote:

7

Nicolas,

Thank you.

I was most interested in the linkage area, ie., the amount of space between TOS and the parameter area. My only concern is being able to locate the frame pointer (when FP != SP) during stack unwinding (debugging and eh.) If r31 is always saved at -4/-8 (i.e/. a constant distance) from previous frame (first saved register) then things should be fine.

Cheers,

-- Jim

8

Hi Chris,

It is marked callee saved. Because when it is not needed as frame pointer it is used like an ordinary register. But when it is used as frame pointer, the prologue and epilogue change its value, but the algorithm in llvm that finds clobbered register does not select it.
    
Okay, I'm not sure. If you describe the constraints better, perhaps Jim or Evan will have an idea :slight_smile:
  

Well i believe the algorithm that finds clobbered registers does not look at the epilogue
and prologue, and therefore does not select R31.
Maybe all I need to do is to explicitly add R31 as a clobbered register when it is used
as a frame pointer. But I need some help to do that.

Thx,
Nicolas

9

Hi Chris,

It is marked callee saved. Because when it is not needed as frame
pointer it is used like an ordinary register. But when it is used as
frame pointer, the prologue and epilogue change its value, but the
algorithm in llvm that finds clobbered register does not select it.

Okay, I'm not sure. If you describe the constraints better, perhaps Jim
or Evan will have an idea :slight_smile:

Well i believe the algorithm that finds clobbered registers does not
look at the epilogue
and prologue, and therefore does not select R31.
Maybe all I need to do is to explicitly add R31 as a clobbered register
when it is used
as a frame pointer. But I need some help to do that.

I am still sure if I understand. R31 is the PPC frame pointer. Are you saying that if frame pointer cannot be eliminated (i.e. hasFP()) is true, you want to spill it to a particular stack slot? You can override the default MRegisterInfo::processFunctionBeforeCalleeSavedScan() implementation and force it to be spilled. Take a look at ARMRegisterInfo.cpp for an example.

Evan

10

Hi Chris,

Chris Lattner wrote:

2) Line 369 of PPCInstrInfo.td, we declare the non-callee saved registers. However, Linux and Darwin do not have the same set
of non-callee saved registers. I don't know how to make the if(isDarwin) test in here
    
Take a look at ARM/ARMRegisterInfo.td for an example of this

I tried to define Defs just like ARMRegisterInfo.td does with different subtargets, but i get the obvious
message:
Value 'Defs' of type 'list<Register>' is incompatible with initializer '[{
(the code is at the end of this mail)

I'm not sure how to play with Defs and what to write in a .td file. I tried a top-level if with a Predicate:
def IsMacho : Predicate<"Subtarget->isMachoABI()">;
if (isMacho) let Defs = ....
else Defs = ...

But this fails too.

Any other ideas on how to get this right?

Thx,
Nicolas

let isCall = 1, noResults = 1, PPC970_Unit = 7,
  // All calls clobber the non-callee saved registers...
  Defs = [{

          F0,F1,F2,F3,F4,F5,F6,F7,F8,F9,F10,
          V0,V1,V2,V3,V4,V5,V6,V7,V8,V9,V10,V11,V12,V13,V14,V15,V16,V17,V18,V19,
          LR,CTR,
          CR0,CR1,CR5,CR6,CR7}
   static const unsigned Defs_Macho = {R0,R2,R3,R4,R5,R6,R7,R8,R9,R10,R11,R12,
          F0,F1,F2,F3,F4,F5,F6,F7,F8,F9,F10,F11,F12,F13,
          V0,V1,V2,V3,V4,V5,V6,V7,V8,V9,V10,V11,V12,V13,V14,V15,V16,V17,V18,V19,
          LR,CTR,
          CR0,CR1,CR5,CR6,CR7}
   GPRClass::iterator
    GPRClass::allocation_order_begin(const MachineFunction &MF) const {
      const TargetMachine &TM = MF.getTarget();
      const PPCSubtarget &Subtarget = TM.getSubtarget<PPCSubtarget>();
      if (Subtarget.isMachoABI()){
        return Defs_Macho;
      } else {
        return Defs_ELF;
      }
    }

    GPRClass::iterator
    GPRClass::allocation_order_end(const MachineFunction &MF) const {
      const TargetMachine &TM = MF.getTarget();
      const MRegisterInfo *RI = TM.getRegisterInfo();
      const PPCSubtarget &Subtarget = TM.getSubtarget<PPCSubtarget>();
      GPRClass::iterator I;
      if (Subtarget.isMachoABI()) {
        I = Defs_Macho + (sizeof(Defs_Macho)/sizeof(unsigned));
      } else {
        I = Defs_ELF + (sizeof(Defs_ELF)/sizeof(unsigned));
      }

      return I;
    }
  }]

  in {
  // Convenient aliases for call instructions
  def BL : IForm<18, 0, 1, (ops calltarget:$func, variable_ops),
                            "bl $func", BrB, >; // See Pat patterns below.
  def BLA : IForm<18, 1, 1, (ops aaddr:$func, variable_ops),
                            "bla $func", BrB, [(PPCcall (i32 imm:$func))]>;
  def BCTRL : XLForm_2_ext<19, 528, 20, 0, 1, (ops variable_ops), "bctrl", BrB,
                           [(PPCbctrl)]>;
}

11

I think the easiest thing for you to do is to define a separate CALL instruction with a different set of Defs. This instruction should only be selected when the predicate isMacho is true. Also update PPCRegisterInfo.cpp getCalleeSavedRegs() to return a different list when subtarget->isMachoABI() is true.

Evan

12

Evan Cheng wrote:

I think the easiest thing for you to do is to define a separate CALL instruction with a different set of Defs. This instruction should only be selected when the predicate isMacho is true. Also update PPCRegisterInfo.cpp getCalleeSavedRegs() to return a different list when subtarget->isMachoABI() is true.
  

Alright, thx Evan, that's what I did.

Here are the final patches I think can be committed. I tried to separate them into independent chunks, but I'm not
sure how to do this in a good way because of CVS and since everything is kind of related.

CallABIELF.patch file changes PPCISelLowering.cpp file for ELF ABI call support
CalleeSavedLinuxPPC.patch changes the callee saved registers for function calls
Creqv.patch adds some XLForm_1 classes to PPCInstrFormats.td for the CREQV instruction support
Frame.patch modifies PPCFrameInfo.h to take into account the ELF ABI for frame manipulation
JITLinuxPPC.patch adds support to detect a Linux/PPC JIT and separate the ELF ABI with the MachO ABI

I didn't sign any licence paper for LLVM. Let me know if I have to do something to commit to CVS. I can also give my code without any
restriction if someone wants to commit.

Cheers,
Nicolas

CallABIELF.patch (13 KB)

CalleeSavedLinuxPPC.patch (9.11 KB)

Creqv.patch (1.36 KB)

Frame.patch (10.7 KB)

JITLinuxPPC.patch (4.29 KB)

13

Sorry for the delay,

I applied the patches after some cleanups. Please keep code within 80 columns, please don't use nested ?: expressions without parens, and please be careful about indentation. Please verify that mainline CVS has everything you think it should.

The one hunk I didn't apply was this one:

@@ -1392,12 +1418,13 @@
      case MVT::f32:
      case MVT::f64:
- if (isVarArg && isPPC64) {
+ if (isVarArg || isPPC64) {
          // Float varargs need to be promoted to double.
          if (Arg.getValueType() == MVT::f32)
            Arg = DAG.getNode(ISD::FP_EXTEND, MVT::f64, Arg);

This changes the darwin/ppc ABI. It's not clear to me that this was intended, so I just left it out.

Thanks a lot for these patches. How well does linux/ppc work now?

-Chris

14

Hi Chris,

Chris Lattner wrote:

Sorry for the delay,

No problem. Plus the reviewing may have taken some time. So thx a lot
for committing. I talked to Jim who said
he wanted to commit his changes before mine -- I hope everything's Ok.

I applied the patches after some cleanups. Please keep code within 80 columns, please don't use nested ?: expressions without parens, and please be careful about indentation.

Alright, sorry I forgot that. I'll be careful next time. Btw, if i have some bug fixes
to do, what should I do? Ask for a write access? Or just do an RFC and waiting for
someone to commit?

Please verify that mainline CVS has everything you think it should.
  

I just verified and launched some compilations. Everything seems OK.

The one hunk I didn't apply was this one:

@@ -1392,12 +1418,13 @@
      case MVT::f32:
      case MVT::f64:
- if (isVarArg && isPPC64) {
+ if (isVarArg || isPPC64) {
          // Float varargs need to be promoted to double.
          if (Arg.getValueType() == MVT::f32)
            Arg = DAG.getNode(ISD::FP_EXTEND, MVT::f64, Arg);

This changes the darwin/ppc ABI. It's not clear to me that this was intended, so I just left it out.
  

Ah yes, I changed this and forgot to mention it (it deserves a different commit - it's, as you
noticed not, directly related to the linux/ppc abi).
I think it should be a "or" instead of an "and" :
floats are promoted to double in a vararg call wether you're on ppc64 or ppc32. Maybe i'm missing
something, anyone can confirm?

Thanks a lot for these patches. How well does linux/ppc work now?

Thanks to you for reviewing and committing.
I would like to say it works great :slight_smile: but I know there is one bug
I have to found because I sometimes seem to get an infinite loop.

If others have the opportunity to test it, please give us some feedback.

Cheers,
Nicolas

15

No problem. Plus the reviewing may have taken some time. So thx a lot
for committing. I talked to Jim who said
he wanted to commit his changes before mine -- I hope everything's Ok.

There is one significant bug that fell out from this, which caused some macho function calls to be compiled with ELF ABI semantics. I checked in this patch to fix it:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070219/045027.html

I think you have to split PPCISD::CALL into PPCISD::CALL_ELF and PPCISD::CALL_Macho, like you do for BCTRL. There were also a couple missing patterns, together these caused some nightly tester failures on darwin.

I applied the patches after some cleanups. Please keep code within 80
columns, please don't use nested ?: expressions without parens, and please
be careful about indentation.

Alright, sorry I forgot that. I'll be careful next time. Btw, if i have some bug fixes to do, what should I do? Ask for a write access? Or just do an RFC and waiting for someone to commit?

You are welcome to have commit-after-approval access. I'll contact you offline. Please familiarize yourself with the developer policy:
http://llvm.org/docs/DeveloperPolicy.html
specifically:
http://llvm.org/docs/DeveloperPolicy.html#commitaccess

Please verify that mainline CVS has
everything you think it should.

I just verified and launched some compilations. Everything seems OK.

Ok, I just broke ELF support to refix darwin, so please take a look again :slight_smile:

The one hunk I didn't apply was this one:

@@ -1392,12 +1418,13 @@
      case MVT::f32:
      case MVT::f64:
- if (isVarArg && isPPC64) {
+ if (isVarArg || isPPC64) {
          // Float varargs need to be promoted to double.
          if (Arg.getValueType() == MVT::f32)
            Arg = DAG.getNode(ISD::FP_EXTEND, MVT::f64, Arg);

This changes the darwin/ppc ABI. It's not clear to me that this was
intended, so I just left it out.

Ah yes, I changed this and forgot to mention it (it deserves a different commit - it's, as you noticed not, directly related to the linux/ppc abi). I think it should be a "or" instead of an "and" : floats are promoted to double in a vararg call wether you're on ppc64 or ppc32. Maybe i'm missing something, anyone can confirm?

I spent quite a bit of time investigating this. As it turns out, llvm-gcc automatically does the promotion before the code generator saw this, so it never could occur before. I changed the check to:

       if (isVarArg) {
         // Float varargs need to be promoted to double.
         if (Arg.getValueType() == MVT::f32)
           Arg = DAG.getNode(ISD::FP_EXTEND, MVT::f64, Arg);
       }

which is correct (non-vararg floats passed to functions on ppc64 don't need extension).

Nice catch.

Thanks a lot for these patches. How well does linux/ppc work now?

Thanks to you for reviewing and committing.
I would like to say it works great :slight_smile: but I know there is one bug
I have to found because I sometimes seem to get an infinite loop.

If others have the opportunity to test it, please give us some feedback.

Ok!

-Chris

16

Chris Lattner wrote:

There is one significant bug that fell out from this, which caused some macho function calls to be compiled with ELF ABI semantics. I checked in this patch to fix it:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070219/045027.html

I think you have to split PPCISD::CALL into PPCISD::CALL_ELF and PPCISD::CALL_Macho, like you do for BCTRL. There were also a couple missing patterns, together these caused some nightly tester failures on darwin.

Yes, you are right. Here's a patch to fix the missing patterns.
There is now a CALL_ELF and a CALL_Macho.

If everything is OK I'll commit it when I'll have write access.

Thanks,
Nicolas

darwin-linux-ppc-CALL.patch (6.64 KB)

17

Patch looks good, go for it!

-Chris