Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PowerPC ncg: register allocation, floats, lxl #59

Merged
merged 32 commits into from
Oct 26, 2017

Conversation

kernigh
Copy link
Contributor

@kernigh kernigh commented Oct 19, 2017

This pull request has several changes to PowerPC ncg. It provides lxl, the EM instruction for following the static chain. It can compare 4-byte floats and convert between 4-byte floats and integers. It enables more ego phases for linuxppc (41f3bf7). The largest changes are to register allocation.

I reduce the number of register properties in PowerPC ncg, so it runs much faster. I explained it in a mail to tack-devel in February 2017. Because of this change, ncg can no longer coerce values to individual registers (except r3), so I must change libem to pass more values on the stack, not in registers. Therefore, everyone must delete and recompile all .o files for PowerPC.

I add floating-point register variables (reg_float), so PowerPC ncg becomes the only ncg to use reg_float. My mail in February 2017 described a problem with 4-byte versus 8-byte floats; I did not fix it until October 2017, when I added the "reglap" feature to ncg. A small change in ego (a20b87c) causes it to put both 4-byte and 8-byte values in reg_float. PowerPC ncg defines overlapping 4-byte and 8-byte registers, and reglap picks the correct size register. I have not written a document to explain reglap.

Two more items about the ego change (a20b87c):

  • It may declare a 4-byte reg_float, like mes 3,-84,4,3,10000, and then put an 8-byte value there, like sdl -84. This works because the PowerPC register can hold both 4-byte and 8-byte values. The offset, like -84, doesn't exist on the stack because ego doesn't allocate stack space for registers.
  • It probably broke the SPARC code expander. We don't build mach/sparc, so I made no attempt to fix it. PowerPC and SPARC are the only platforms with reg_float.

Remove one addi instruction from some loops.  These loops had
increased 2 pointers, they now increase 1 index.  I must initialize
the index, so I add "li r6, 0" before each loop.

Change .zer to use subf instead of neg, add.

Change .xor to take the size on the real stack, as .and and .or have
done since 81c677d.
This replaces a call to memmove() in libc.  That was working for me,
but it can fail because EM programs don't always link to libc.

blm and bls only need to copy aligned words.  They don't need to copy
bytes, and they don't need to copy between overlapping buffers, as
memmove() does.  So the new loop is simpler than memmove().
Switch some conversions from libem calls to inline code.  The
conversions from integers to floats are now too slow, because each
conversion allocates 4 or 5 registers, and the register allocator is
too slow.  I might use these slow conversions to experiment with the
register allocator.

I add the missing conversions between 4-byte single floats and
integers, simply by going through 8-byte double floats.  (These
replace the calls to nonexistant functions in libem.)

I remove the placeholder for fef 4, because it doesn't exist in libem,
and our language runtimes only use fef 8.
Reorder the code in .fef8 and .fif8 so that in the usual case, we fall
through to the blr without taking any branches.  The usual case, by my
guess, is .fef8 with normalized numbers or .fif8 with small integers.

I change .fef8 and .fif8 to pass values on the real stack, not in
specific registers.  This simplifies the ncg table, and might help me
experiment with changes to the ncg table.

This change might or might not help mcg.  Seems that mcg always uses
the stack to pass values to libem, but I have not tested .fef8 or
.fif8 with mcg.
Our libem had two implementations of loading a block from a stack, one
for lar 4 and one for los 4.  Now lar 4 and los 4 share the code in
.los4.  Likewise, sar 4 and sts 4 share the code in .sts4.

Rename .los to .los4 and .sts to .sts4, because they implement los 4
and sts 4.  Remove the special case for loading or storing 4 bytes,
because we can do it with 1 iteration of the loop.  Remove the lines
to "align size" where the size must already be a multiple of 4.

Fix the upper bound check in .aar4.

Change .aar4, .lar4, .los4, .sar4, .sts4 to pass all operands on the
real stack, except that .los4 and .sts4 take the size in register r3.
Have .aar4 set r3 to the size of the array element.  So lar 4 is just
.aar4 then .los4, and sar 4 is just .aar4 then .sts4.

ncg no longer calls .lar4 and .sar4 in libem, because it inlines the
code; but I keep .lar4 and .sar4 in libem, because mcg references
them.  They might or might not work in mcg.
Switch .cms to pass inputs and outputs on the real stack, not in
registers; like we do with .and, .or (81c677d) and .xor (c578c49).

At this point, nearly all functions in libem use the real stack, not
registers, for passing inputs and outputs.  This simplifies the ncg
table (which needs fewer lists of specific registers) but slows calls
to libem.

For example, after ba9b021, each call to .aar4 is about 10
instructions slower.  I moved 3 inputs and 1 output from registers to
the real stack.  A program would take 4 instructions to move registers
to stack, 4 to move stack to registers, and perhaps 2 to adjust the
stack pointer.
The table for PowerPC had placed each GPR and FPR into an individual
register class (like GPR3, GPR4, FPR1, FPR2), and had used these
classes to coerce stack values into specific registers.  But ncg does
not like having many register classes.

In http://tack.sourceforge.net/olddocs/ncg.pdf
Hans van Staveren wrote:

> Every extra property means the register set is more unorthogonal and
> *cg* execution time is influenced by that, because it has to take
> into account a larger set of registers that are not equivalent.  So
> try to keep the number of different register classes to a minimum.

Recent changes to the PowerPC table have removed many coercions to
specific registers.  Many functions in libem switched from taking
values in registers to taking them from the stack (see dc05cb2).

I now remove all 64 individual register classes of GPR and FPR.  In
the few cases where I need a stack value in a specific register, I now
do a move (as the arm and m68020 tables do).

This commit speeds the compilation of some files.  For my test file
fconv.c, the compilation time goes from over 20 seconds to under 1
second.  My fconv.c has 4 conversions from floats to integers, and the
table has my experimental rules that do the conversions by allocating
4 or 5 registers.
I added REG_PAIR in cfbc537 to speed up the register allocator,
because ncg was taking about 2 seconds on each sti 8.  I defined only
4 such pairs, so allocating REG_PAIR was much faster than allocating
REG REG.

After my last commit c5bb3be, allocation of REG REG is fast, and
REG_PAIR seems unnecessary.
This fixes lxl 1 (so it follows the static chain, not the dynamic
chain) and provides lxl 2 and greater.  The Modula-2 compiler uses lxl
for nested procedures, so they can access the variables of the
enclosing procedures.
Use f14 to f31 as register variables for 8-byte double-precison.
There are no regvars for 4-byte double precision, because all
regvar(reg_float) must have the same size.  I expect more programs to
prefer 8-byte double precision.

Teach mach/powerpc/ncg/mach.c to emit stfd and lfd instructions to
save and restore 8-byte regvars.  Delay emitting the function prolog
until f_regsave(), so we can use one addi to make stack space for both
local vars and saved registers.  Be more careful with types in mach.c;
don't assume that int and long and full are the same.

In ncg table, add f14 to f31 as register variables, and some rules to
use them.  Add rules to put the result of fadd, fsub, fmul, fdiv, fneg
in a regvar.  Without such rules, the result would go in a scratch
FREG, and we would need fmr to move it to the regvar.  Also add a rule
for pat sdl inreg($1)==reg_float with STACK, so we can unstack the
value directly into the regvar, again without a scratch FREG and fmr.

Edit util/ego/descr/powerpc.descr to tell ego about the new float
regvars.  This might not be working right; ego usually decides against
using any float regvars, so ack -O1 (not running ego) uses the
regvars, but ack -O4 (running ego) doesn't use the regvars.

Beware that ack -mosxppc runs ego using powerpc.descr but -mlinuxppc
and -mqemuppc run ego without a config file (since 8ef7c31).  I am
testing powerpc.descr with a local edit to plat/linuxppc/descr to run
ego with powerpc.descr there, but I did not commit my local edit.
The size of a reg_float isn't in the descr file, so ego doesn't know.
PowerPC and SPARC are the only arches with floating-point registers in
their descr files.  PowerPC and SPARC registers can hold both 4-byte
and 8-byte floats, so I want ego to do both sizes.

This might break our SPARC code expander because ego doesn't know that
8-byte values take 2 registers in SPARC.  (So ego might allocate too
many registers and deallocate too much stack space.)  We don't build
the SPARC code expander, and its descr file is already wrong: its list
of register save costs is too short, so ego will read past the end of
the array.

This commit doesn't fix the problem with ego and PowerPC ncg.  Right
now, ncg refuses to put 4-byte floats in registers, but ego expects
them to get registers and deallocates their stack space.  So ncg emits
programs that use the deallocated space, and the values of 4-byte
floats become corrupt.
This is like David Given's change to util/ncgg in d89f172.  I need
this change in mach/proto/ncg to see fatal messages, because a 64-bit
pointer doesn't fit in an int.
With this type check, I can change the size checks into assertions.
This removes a wrong-way dependency of libsys on libem.  The C
functions in libsys called .ret, but libsys is after libem in the
linker arguments, so the linker didn't find .ret unless something else
had called .ret.  Almost everything called .ret, but I got a linker
error when I wrote an assembly program using the EM runtime, because
my assembly program didn't call .ret.

Add a dummy comment to build.lua, so git checkout touches that file,
the build system reconfigures itself, and the *.s glob sees that ret.s
has gone.
I broke it in f64b7d8.  My stack pattern had the wrong type of
registers.  The comparison popped too many bytes and corrupted the
stack.
After c5bb3be, ncg began to allocate regvars from r13 up.  I reorder
the regvars so ncg again allocates them from r31 down.  I also reorder
the other registers.

This exposed a bug in my rule for ret 8.  It was wrong if item %2 was
in r3, because I moved %1 to r3 before %2 to r4.  Fix it by adding
back an individual register class for r3 (called REG3 here, GPR3 in
c5bb3be).

Also fix my typo in mach.c that made a syntax error in assembly.
The new feature "reglap" allows two sizes of floating-point register
variables (reg_float), if each register overlaps a single register of
the other size.  PowerPC ncg uses reglap to define 4-byte instances
of f14 to f31 that overlap the 8-byte instances.

When ncgg sees the definition of fs14("f14")=f14, it removes the
8-byte f14 from its rvnumbers array, and adds the 4-byte fs14 in its
place.  Later, when ncg puts a variable in fs14, if it is an 8-byte
variable, then ncg switches to the 8-byte f14.  The code has
/* reglap */ comments in util/ncgg or #ifdef REGLAP in mach/proto/ncg

reglap became necessary because my commit a20b87c caused PowerPC ego
to allocate reg_float in both 4-byte and 8-byte sizes.
This merges several fixes and improvements from upstream.  This
includes commit 5f6a773 to turn off qemuppc.  I see several failing
tests from qemuppc; this merge will hide the test failures.
After the RA phase of ego, a procedure may put single-word and
double-word values in the same reg_float.  Then ncg will use both
LOCAL and DLOCAL tokens at the same offset.

I add isregvar_size() to ncg.  It receives the size of the LOCAL or
DLOCAL token, and picks the register of the correct size.  This fixes
a problem where ncg got the wrong-size register and corrupted the
stack.  This problem caused one of my test programs to segfault from
stack underflow.

Also adjust how fixregvars() handles both sizes.
If the ncg table uses reglap, then regvar($1, reg_float) would have
two sizes of registers.  An error from ncgg would happen if regvar()
was in a token that allows only one size.  Now one can pick a size
with regvar_w() for word size or regvar_d() for double-word size.

Add regvar_d and regvar_w as keywords in ncgg.  Modify EX_REGVAR to
include the register size.  In ncg, add some checks for the register
size.  In tables without reglap, regvar() works as before, and ncg
ignores the register size in EX_REGVAR.
Rename GPRE to GPR_EXPR, then define FPR_EXPR and FSREG_EXPR.  Use
them for moves to register variables.

Keep "kills regvar($1)", because deleting it and recompiling libc
would cause many failures in my test programs.  Add comment to warn,
  /* ncg fails to infer that regvar($1) is dead! */

Remove "kills LOCAL %off==$1" because it seems to have no effect.
Do the conversion by calling .cif8 or .cuf8 in libem, as it was done
before my commit 1de1e8f.  I used the inline conversion to experiment
with the register allocator, which was too slow until c5bb3be.

Now that libem has the only copy of the code, move some comments and
code changes there.
The result of single-precision fadds, fsubs, and such can go into a
register variable, like we already do with double precision.  This
avoids an extra fmr from a temporary register to the regvar.
Because lwzu or stwu moves the pointer, I can remove an addi
instruction from the loop, so the loop is slightly faster.

I wrote a benchmark in Modula-2 that exercises some of these loops.  I
measured its time on my old PowerPC Mac.  Its user time decreases from
8.401s to 8.217s with the tighter loops.
ack -mlinuxppc -O4 now runs more phases of ego, including the register
allocation phase, so ncg emits better code.

Set MACHOPT_F=-m3 as I did it for osxppc; see commit 0c2b6f5.

Remove CC_ALIGN=-Vr so bitfields agree with gcc for PowerPC Linux.

Remove unused C_LIB and OLD_C_LIB.
This relocation is specific to PowerPC.  @davidgiven suggested the
name RELOPPC_LIS in
davidgiven#52 (comment)

Reindent the list in h/out.h and util/led/ack.out.5 because
RELOLIS_PPC is a long name.  I use spaces and no tabs because the tabs
looked bad in the manual page.
In util/ncgg, add two more errors for tables using reglap:
 - "Two sizes of reg_float can't be same size"
 - "Missing reg_float of size %d to contain %s"

In mach/proto/ncg, rename macro isregvar_size() to PICK_REGVAR(), so
the macro doesn't look like a function.  This macro sometimes doesn't
evaluate its second argument.

In mach/powerpc/ncg/mach.c, change type of lfs_set to uint32_t, and
change the left shifts from 1U<<regno to (uint32_t)1<<regno, because
1U would be too small for machines with 16-bit int.
@davidgiven
Copy link
Owner

That's a lot of work, cheers!

The register variable addon to ncg isn't great --- I never made it work acceptably, and more or less gave up. If you can make it work, that's good, but I'd be careful about sinking too much work into it. The ego issue where it assumes that certain variables can always go into registers and doesn't allocate stack for them makes me a bit nervous --- it'd be nice if we could at least make it give them invalid stack IDs so that the code generator could abort if it saw one, rather than just generating bad code...

Also, the libem ABI changes break mcg. As mcg's not ready (I got burnt out on implementing complex data structures in C and did something else) that's not a problem, but could you make sure that mcg isn't built and file a bug, please?

What's the generated code quality like now?

@kernigh
Copy link
Contributor Author

kernigh commented Oct 21, 2017

I entered bug #61 for mcg.

The code quality from PowerPC ncg seems correct but slow. If I would write PowerPC assembly by hand, my code might be faster than ncg, but slower than gcc and clang. That's because gcc and clang know more tricks. Both gcc and clang might reorder instructions to speed the pipeline. Now ack has no instruction scheduler, and ncg tends to emit code that writes a register immediately before reading it, which might stall the pipeline when the load waits for the completion of the store.

I present this C function integrate() and its ncg code.

struct df {
	double d;
	float f;
};

/*
 * Find the approximate area under the function f in the interval
 * [a, b] by summing the area of many rectangles.
 */
struct df
integrate(double a, double b, double (*f)(double))
{
	struct df area;
	double td, xd;
	float af, bf, tf, xf;
	const int rects = 1000;
	int i;

	af = a;
	bf = b;
	td = 0;
	tf = 0;
	for (i = 0; i < rects; i++) {
		/* using double */
		xd = a + (b - a) * (2 * i + 1) / (double)(2 * rects);
		td += f(xd) * (b - a) / (double)rects;
		/* using float */
		xf = af + (bf - af) * (2 * i + 1) / (float)(2 * rects);
		tf += (float)f(xf) * (bf - af) / (float)rects;
	}
	area.d = td;
	area.f = tf;
	return area;
}

It is part of a larger program, compiled with ack -mosxppc -O4 -c.s integral.c. With -O4, ego optimizes the EM code so ncg has more opportunities to use register variables. This seems to decrease size of code, and improve quality.

Our ncg table has better rules for stl (store local) when storing to a regvar, not the stack. For a regvar, we write the value and kill the register. For the stack, we rewrite stl as sti (store indirect), then write the value and kill all MEMORY tokens. This might emit extra code to move MEMORY tokens to the stack.

_integrate:
mfspr r0, lr
addi sp, sp, -164
stw fp, 156(sp)
stw r0, 160(sp)
addi fp, sp, 156
stfd f31, -80(fp)
stfd f30, -88(fp)
stfd f29, -96(fp)
stfd f28, -104(fp)
stfd f27, -112(fp)
stfd f26, -120(fp)
stfd f25, -128(fp)
stfd f24, -136(fp)
stmw r27, -156(fp)

integrate() has 13 register variables (in f31 to f24, and r31 to r27). My numbers in util/ego/descr/powerpc.descr are less than accurate. I set the "register save costs" for 13 registers to 78 cycles, 104 bytes. This assumes 13 stw and 13 lzw instructions (in the MPC7447A processor), but integrate() uses different instructions. Also, we may have misalignment because stfd wants 8-byte alignment but ncg provides 4-byte alignment for sp and fp.

frsp f1,f26
fmr f28,f1
frsp f2,f24
fmr f25,f2

This uses frsp to convert double to single precision, but I forgot to add a rule to frsp into a regvar. So ncg must frsp into a temporary register (f1 or f2), then fmr to the regvar (f28 or f25).

b I1_1
I1_3:
addi r4,r31,1
mr r30,r4
stwu r4,-4(sp)
bl .cif8

This is the top of the loop. We addi into temporary r4, then mr to regvar r30. We don't need r4 because .cif8 kills r4. EM code has inc, stl -124, lol -124. The table can do inc, stl -124 without a temporary. It doesn't, because the table rewrites stl -124, lol -124, as dup 4, stl -124, and the rule for dup 4 uses a temporary.

li r4,2000
mr r29,r4
stfdu f2,-8(sp)
stwu r4,-4(sp)
bl .cif8
lfd f1,0(sp)
addi sp,sp,8
lfd f2,0(sp)
addi sp,sp,8
fdiv f1,f2,f1
fadd f30,f1,f26
stfdu f30,-8(sp)
mtspr ctr,r27
bctrl

This is doing xd = a + ... / (double)(2 * rects); ... f(xd) ...;. The compiler folded 2 * rects into a const int 2000, but the conversion to double 2000.0 happens at runtime. Worse, the conversion is inside a loop that runs 1000 times. Also, the conversion in .cif8 does too much work. It allocates 8 bytes of stack to hold [0x4330 << 16, 2000 ^ (1 << 31)] and a temporary f1 = (1 << 52) + (1 << 31). I would keep an 8-byte local buffer with 0x4330 << 16 in the first word, and a regvar with (1 << 52) + (1 << 31), and reuse them in conversions; but ncg can't allocate local buffers or regvars that don't exist in the EM code.

Also, putting mtspr ctr,r27 just before bctrl might stall the pipeline. It might help to set ctr earlier. The call to .cif8 might destroy ctr, but we can set ctr after .cif8 returns.

fadds f29,f1,f29
addi r28,r28,1
addi r31,r31,2
I1_1:
cmpwi r28,1000
blt I1_3

This is the bottom of the loop. It continues if r28 < 1000. cmpwi sets cr0 and blt reads cr0, perhaps stalling the pipeline, but we can't move cmpwi before the label. If the processor has branch prediction, it might predict branch taken, and be correct 999 of 1000 times.

stfd f27,-16(fp)
stfs f29,-8(fp)
li r3,16
addi r0,fp,-16
stwu r0,-4(sp)
bl .los4
li r3,16
lwz r0,8(fp)
stwu r0,-4(sp)
bl .sts4

This does area.d = td; area.f = tf; and prepares to return area;. There is a pointer in 8(fp) to the 16-byte struct being returned. The code should just lwz r3,8(fp), stfd f27,0(r3), stfs f29,8(r3), and be done. Here ncg is writing the struct in a local variable at -8(fp), then using .los4 to copy the struct to the stack, then using .sts4 to copy it from the stack to the return value.

@davidgiven
Copy link
Owner

I'm actually really impressed that you've got it producing code this good --- given that the ACK doesn't really believe that registers exist, I never would have thought it possible!

@davidgiven davidgiven merged commit 6614752 into davidgiven:default Oct 26, 2017
@kernigh kernigh deleted the kernigh-ppc-regs branch October 27, 2017 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants