The strength reduction (SR) phase mishandled the END pseudo when SR
found at least 2 different expressions in the same loop, and SR made a
new header for the loop.
Since 1987 (commit 159b84e), SR appended the new header to the end of
the procedure. Later, SR fixed the header by adding a BRA branch to
the loop's entry, and moving the END pseudo from the previous basic
block to the header. If SR found multiple expressions, it called
fix_header() multiple times, and tried to move the END again. The
extra move failed assert(INSTR(e) == ps_end), or moved another line
after the END, so opt2 (the peephole optimizer) would crash.
Fix by removing fix_header() and moving the code to make_header(), so
SR adds the BRA and moves the END when it makes the header, and does
so only once for each header.
Adjust init_code(), which inserts code into a header. Stop checking
for an empty header, because make_header() now adds BRA. After
inserting code before BRA, don't move LP_INSTR, because later
insertions should go before BRA, not after END.
Our build enables assertions in some other ACK tools (like assemblers
and LLgen), but disabled them in ego until now. Bug #203 becomes a
failed assertion in ego's SR phase:
sr: util/ego/sr/sr_reduce.c:483: fix_header: Assertion
`INSTR(e) == ps_end' failed.
Comment out 2 assertions in util/ego/share, because they fail on
systems with 4-byte int, 8-byte long.
Most warnings are for functions implicitly returning int. Change most
of these functions to return void. (Traditional K&R C had no void
type, but C89 has it.)
Add prototypes to most function declarations in headers. This is
easy, because ego declares most of its extern functions, and the
comments listed most parameters. There were a few outdated or missing
declarations, and a few .c files that failed to include an .h with the
declarations.
Add prototypes to a few function definitions in .c files. Most
functions still have traditional K&R definitions. Most STATIC
functions still don't have prototypes, because they have no earlier
declaration where I would have added the prototype.
Change some prototypes in util/ego/share/alloc.h. Functions newmap()
and oldmap() handle an array of pointers to something; change the
array's type from `short **` to `void **`. Callers use casts to go
between `void **` and the correct type, like `line_p *`. Function
oldtable() takes a `short *`, not a `short **`; I added the wrong type
in 5bbbaf4.
Make a few other changes to silence warnings. There are a few places
where clang wants extra parentheses in the code.
Edit util/ego/ra/build.lua to add the missing dependency on ra*.h; I
needed this to prevent crashes from ra.
Edit C code to reduce warnings from clang. Most warnings are for
implicit declarations of functions, but some warnings want me to add
parentheses or curly braces, or to cast arguments for printf().
Make a few other changes, like declaring float_cst() in h/con_float to
be static, and using C99 bool in ego/ra/makeitems.c and
ego/share/makecldef.c. Such changes don't silence warnings; I make
such changes while I silence warnings in the same file. In
float_cst(), rename parameter `str` to `float_str`, so it doesn't
share a name with the global variable `str`.
Remove `const` from `newmodule(const char *)` in mach/proto/as to
silence a warning. I wrongly added the `const` in d347207.
For warnings about implicit declarations of functions, the fix is to
declare the function before calling it. For example, my OpenBSD
system needs <sys/wait.h> to declare wait().
In util/int, add "whatever.h" to declare more functions. Remove old
declarations from "mem.h", to prefer the newer declarations of the
same functions in "data.h" and "stack.h".
CS eliminates outer expressions before inner ones, as `x * y * z`
before `x * y`. It does this by reversing the order of expressions in
the code. This almost always works, but it sometimes doesn't work if
a STI changes the value number of a LOI. In code like `expr1 LOI
expr2 STI expr2 LOI`, CS might eliminate the inner `expr2` before the
outer `expr2 LOI`. This caused a read after free because the
occurrence of `expr2 LOI` pointed to the eliminated lines of `expr2`.
This bug went unnoticed until my recent changes caused CS to crash
with a double free. I did not get the crash in OpenBSD, but I saw the
crash in Travis, then David Given reproduced the crash in Linux. See
the discussion in https://github.com/davidgiven/ack/pull/73
Enable this in CS for PowerPC; disable it for all other machines.
PowerPC has no remainder instruction; the back end uses division to
compute remainder. If CS finds both a / b and a % b, then CS now
rewrites a % b as a - b * (a / b) and computes a / b only once. This
removes an extra division in the PowerPC code, so it saves both time
and space.
I have not considered whether to enable this optimization for other
machines. It might be less useful in machines with a remainder
instruction. Also, if a % b occurs before a / b, the EM code gets a
DUP. PowerPC ncg handles this DUP well; other back ends might not.
In ego, the CS phase may convert a LAR/SAR to AAR LOI/STI so it can
optimize multiple occurrences of AAR of the same array element. This
conversion should not happen if it would LOI/STI a large or unknown
size.
cs_profit.c okay_lines() checked the size of each occurrence of AAR
except the first. If the first AAR was the implicit AAR in a LAR/SAR,
then the conversion happened without checking the size. For unknown
size, this made a bad LOI -1 or STI -1. Fix by checking the size
earlier: if a LAR/SAR has a bad size, then don't enter it as an AAR.
This Modula-2 code showed the bug. Given M.def:
DEFINITION MODULE M;
TYPE S = SET OF [0..95];
PROCEDURE F(a: ARRAY OF S; i, j: INTEGER);
END M.
and M.mod:
(*$R-*) IMPLEMENTATION MODULE M;
FROM SYSTEM IMPORT ADDRESS, ADR;
PROCEDURE G(s: S; p, q: ADDRESS; t: S); BEGIN
s := s; p := p; q := q; t := t;
END G;
PROCEDURE F(a: ARRAY OF S; i, j: INTEGER); BEGIN
G(a[i + j], ADR(a[i + j]), ADR(a[i + j]), a[i + j])
END F;
END M.
then the bug caused an error:
$ ack -mlinuxppc -O3 -c.e M.mod
/tmp/Ack_b357d.g, line 57: Argument range error
The bug had put LOI -1 in the code, then em_decode got an error
because -1 is out of range for LOI.
Procedure F has 4 occurrences of `a[i + j]`. The size of `a[i + j]`
is 96 bits, or 12 bytes, but the EM code hides the size in an array
descriptor, so the size is unknown to CS. The pragma `(*$R-*)`
disables a range check on `i + j` so CS can work. EM uses AAR for the
2 `ADR(a[i + j])` and LAR for the other 2 `a[i + j]`. EM pushes the
arguments to G in reverse order, so the last `a[i + j]` in Modula-2 is
the first LAR in EM.
CS found 4 occurrences of AAR. The first AAR was an implicit AAR in
LAR. Because of the bug, CS converted this LAR 4 to AAR 4 LOI -1.
- In share/debug.c, undo my mistake in commit 9037d13 by changing
vfprintf back to fprintf in OUTTRACE.
- In ud/ud.c, move the trace output from stdout to stderr, because
stdout has ego's output file, which becomes opt2's input file. If
trace output goes to stdout, it gets prepended to the output file,
and opt2 errors with "wrong input file".
I also edit both build.lua files so ego depends on its header files;
this part isn't needed for -DTRACE.
One can now use -DTRACE by adding it to the cflags in both build.lua
files.
This uncovers a problem in il/il_aux.c: it passes 3 arguments to
getlines(), but the function expects 4 arguments. I add FALSE as the
4th argument. TRUE would fill in the list of mesregs. IL uses
mesregs during phase 1, but this call to getlines() is in phase 2.
TRUE would leak memory unless I added a call to Ldeleteset(mesregs).
So I pass FALSE.
Functions passed to go() now have a `void *` parameter because
no_action() now takes a `void *`.
*Important:* Do `make clean` to work around a problem and prevent
infinite rebuilds, https://github.com/davidgiven/ack/issues/68
I edit tokens.g in util/LLgen/src, so I regenerate tokens.c. The
regeneration script bootstrap.sh can't find LLgen, but I can run the
same command by typing the path to llgen.
@dram reported a build failure in FreeBSD at
https://github.com/davidgiven/ack/issues/1#issuecomment-273668299
Linux manual for getopt(3) says:
> If the first character of optstring is '-', then each nonoption
> argv-element is handled as if it were the argument of an option with
> character code 1....
>
> The use of '+' and '-' in optstring is a GNU extension.
GNU/Linux and OpenBSD handle '-' in this special way, but FreeBSD
seems not to. If '-' is not special, then em_ego can't find its input
file, so the build must fail. This commit stops using '-' in both
em_b and em_ego, but doesn't change mcg.
Also fix em_ego -O3 to not act like -O4.
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.
same base name and generate multiple files based on it, we can't really use
mkstemp() for every temporary file. Instead, use mkstemp() once on a
placeholder, then generate temporary names based on this. (And delete the
placeholder once we've finished.)
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.
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.
No change to linuxppc and qemuppc. They continue to run ego without
any descr file.
I copied m68020.descr to powerpc.descr and changed some numbers. My
numbers are guesses; I know little about PowerPC cycle counts, and
almost nothing about ego. This powerpc.descr causes most of the
example programs to shrink in size (without descr -> with descr):
65429 -> 57237 hilo_b.osxppc -8192
36516 -> 32420 hilo_c.osxppc -4096
55782 -> 51686 hilo_mod.osxppc -4096
20096 -> 20096 hilo_p.osxppc 0
8813 -> 8813 mandelbrot_c.osxppc 0
93355 -> 89259 paranoia_c.osxppc -4096
92751 -> 84559 startrek_c.osxppc -8192
(Each file has 2 Mach segments, then a symbol table. Each segment
takes a multiple of 4096 bytes. When the code shrinks, we lose a
multiple of 4096 bytes.)
I used "ack -mosxppc -O6 -c.so" to examine the assembly code for
hilo.mod and mandelbrot.c, both without and with descr. This reveals
optimizations made only with descr, from 2 ego phases: SP (stack
pollution) and RA (register allocation). In hilo.mod, SP deletes some
instructions that remove items from the stack. These items get
removed when the function returns. In both hilo.mod and mandelbrot.c,
RA moves some values into local variables, so ncg can make them into
register variables. This shrinks code size, probably because register
variables get preserved across function calls. More values stay in
registers, and ncg emits shorter code.
I believe that the ego descr file uses (time,space) tuples but the ncg
table uses (space,time) tuples. This is confusing. Perhaps I am
wrong, and some or all tuples are backwards. My time values are the
cycle counts in latency from the MPC7450 Reference Manual (but not
including complications like "store serialization").
In powerpc.descr, I give the cost for saving and restoring registers
as if I was using chains of stw and lwz instructions. Actually ncg
uses single stmw and lmw instructions with at least 2 instructions.
The (time,space) for stmw and lmw would be much less than the
(time,space) for chains of stw and lwz. But this ignores the pipeline
of the MPC7450. The chains of stw and lwz may run faster than stmw
and lmw in the pipeline, because the throughput may be better than the
latency. By using the wrong values for (time,space), I'm trying to
tell ego that stmw and lmw are not better than chains of stw and lwz.
cf/cf_loop.c and share/put.c tried to read the next pointer in an
element of a linked list after freeing the element. ud/ud_copy.c
tried to read beyond the end of the _defs_ array: it only has
_nrexpldefs_ elements, not _nrdefs_ elements.
These bugs caused core dumps on OpenBSD. Its malloc() put _defs_ near
the end of a page, so reading beyond the end crossed into an unmapped
page. Its free() wrote junk bytes and changed the next pointer to
0xdfdfdfdfdfdfdfdf.
and generate invalid calls to the optimisers.
Previously ego would generate a temporary file template that looked like
/tmp/ego.A.BB.XXXXXX, call mktemp() on it to randomise the XXXXXX, and then
replace A and BB with data.
However, it used strrchr to find the A and B. Which would fine, except when
mktemp produced an A or a B in the randomised part...
This code was written on 4 March 1991. I was 16.
This needed lots of refactoring to ego --- not all platforms have ego descr
files, and ego will just crash if you invoke it without one. I think originally
it was never intended that these platforms would be used at -O2 or above.
Plats now only specify the ego descr file if they have one.
These files "magically reappeared" after the conversion from CVS to
Mercurial. The old CVS repository deleted these files but did not
record *when* it deleted these files. The conversion resurrected these
files because they have no history of deletion. These files were
probably deleted before year 1995. The CVS repository begins to record
deletions around 1995.
These files may still appear in older revisions of this Mercurial
repository, when they should already be deleted. There is no way to fix
this, because the CVS repository provides no dates of deletion.
See http://sourceforge.net/mailarchive/message.php?msg_id=29823032
