Patch - Big stacks on SPU, take 2

Hi,

attached is a second try for the bigstack patch for SPU, with testcase. It is essentially the patch committed as 97091, and reverted as 97099, but with the following additions:
-in vararg handling, registers are marked to be live, to not confuse the register scavenger
-function prologue and epilogue are not emitted, if the stack size is 16. 16 means it is empty - there is only the register scavenger emergency spill slot, which is not used as there is no stack.

This time there are no new unexpected failures in the regression tests.

kalle

spu_bigstack.patch (10.3 KB)

bigstack.ll (471 Bytes)

works for me, applied here, thanks!
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20100329/098759.html

attached is a second try for the bigstack patch for SPU, with testcase. It is essentially the patch committed as 97091, and reverted as 97099, but with the following additions:
-in vararg handling, registers are marked to be live, to not confuse the register scavenger

Looks good. You can try running with -verify-machineinstrs to detect more issues like that.

-function prologue and epilogue are not emitted, if the stack size is 16. 16 means it is empty - there is only the register scavenger emergency spill slot, which is not used as there is no stack.

Would it be possible to detect this before allocating the emergency spill slot, and not request a scavenger at all?

This time there are no new unexpected failures in the regression tests.

I noticed that CellSPU has a bunch of isS10Constant() functions. These already exist in MathExtras.h: isInt<10>(). Also, there is no need for 5 overloads of those functions, AFAICT.

Please use the MathExtras.h functions, and remove all of the is*Constant functions in SPU.h.

/jakob

Jakob Stoklund Olesen skrev:

Looks good. You can try running with -verify-machineinstrs to detect more
issues like that.

Didn't know about that option, thanks for the tip. The output looks useful!

-function prologue and epilogue are not emitted, if the stack size is 16.
16 means it is empty - there is only the register scavenger emergency spill
slot, which is not used as there is no stack.

Would it be possible to detect this before allocating the emergency spill
slot, and not request a scavenger at all?

Initially I thought not, but heeding your previous advice and studying the ARM backend closely, I noticed the function estimateStackSize... I copied the function to the SPU backend, and enable the scavenger conditionally if stack is small enough. Initial tests look promising!

The estimateStackSize looks quite generic (backend ignorant). Is there a reason it is in local to the ARM and not in TargetRegisterInfo class? Are there cases when the estimation would fail?

I noticed that CellSPU has a bunch of isS10Constant() functions. These
already exist in MathExtras.h: isInt<10>(). Also, there is no need for 5
overloads of those functions, AFAICT.

Hmm, I just mindlessly copied some legacy code. Thanks for the tip.
I noticed that Benjamin Kramer already made the changes to the SPU backend :slight_smile:

kalle