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
the -U command line option, and one via file scanning. Turns out only the
second would increment the number of global names, so adding names with -U
would cause names found via scanning to fall off the end of the list! This
wouldn't cause linker errors because fixups don't use the list, but would cause
the generated symbol table in the output to be incorrect.
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.
I made a syntax error in some .e file, and em_encode dumped core
because a 64-bit pointer didn't fit in a 32-bit int. Now use stdarg
to pass pointers to error() and fatal().
Stop using the number of errors as the exit status. Many systems use
only the low 8 bits of the exit status, so 256 errors would become 0.
Also change modules/src/print to accept const char *buf
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.
Silence warning from clang at `if (ch2 = ...)`
Delete `|| rm %{outs}` in build.lua, because it hid the exit status of
tabgen, so if tabgen failed, the build continued and failed later.
Edit build.lua for programs losing their private assert.h, so they
depend on a list of .h files excluding assert.h.
Remove modules/src/assert; it would be a dependency of cpp.ansi but we
didn't build it, so cpp.ansi uses the libc assert.
I hope that libc <assert.h> can better report failed assertions. Some
old "assert.h" files didn't report the expression. Some reported a
literal "x", because traditional C expanded the macro parameter x in
"x", but ANSI C89 doesn't expand macro parameters in string literals.
Because of the accidental deletion, mcgg on my machine followed a
garbage pointer, and never wrote calls to emit_fragment.
A wrong call to `data->emit_reg(0, 0)` instead of the correct
`data->emit_fragment(0)` caused PowerPC mcg to emit an empty string
instead of `8(fp)`, causing a syntax error in PowerPC as.
The wrong `data->emit_reg(0, 0)` called the function emit_reg() in
mach/proto/mcg/pass_instructionselection.c, but that function
unfortunately has `if (vreg) { ... }`. The call had vreg == NULL
because the fragment wasn't a vreg, but emit_reg() ignored the problem
and emit nothing.
@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 is more useful when looking for patterns; lino - 1 is probably
the line number in the patterns file. DIAGOPT is off by default but
one can edit optim.h to enable it.
The other changes just clean up whitespace.
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.
This relocation is specific to PowerPC. @davidgiven suggested the
name RELOPPC_LIS in
https://github.com/davidgiven/ack/pull/52#issuecomment-279856501
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.
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.
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.
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.
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.
I hastily chose the name RELOLIS for this relocation type. If we want
to rename it, we only need to edit these files:
- h/out.h
- mach/powerpc/as/mach5.c
- util/amisc/ashow.c
- util/led/ack.out.5
- util/led/relocate.c
The new relocation type RELOLIS handles these instructions:
lis RT, ha16[expr] == addis RT, r0, ha16[expr]
lis RT, hi16[expr] == addis RT, r0, hi16[expr]
RELOLIS stores a 32-bit value in the program text. In this value, the
high bit is a ha16 flag, the next 5 bits are the target register RT,
and the low bits are a signed 26-bit offset. The linker replaces this
value with the lis instruction.
The old RELOPPC relocated a ha16/lo16 or hi16/lo16 pair. The new
RELOLIS relocates only a ha16 or hi16, so it is no longer necessary to
have a matching lo16 in the next instruction. The disadvantage is
that RELOLIS has only a signed 26-bit offset, not a 32-bit offset.
Switch the assembler to use RELOLIS for ha16 or hi16 and RELO2 for
lo16. The li32 instruction still uses the old RELOPPC relocation.
This is not the same as my RELOPPC change from my recent mail to
tack-devel (https://sourceforge.net/p/tack/mailman/message/35651528/).
This commit is on a different branch. Here I am throwing away my
RELOPPC change and instead trying RELOLIS.
