d0p1/ack
1
0
Fork 0
Code Issues Pull requests Packages Projects Releases Wiki Activity

Check AAR earlier to prevent LOI/STI unknown size.

Browse source 
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.
This commit is contained in:
George Koehler 2018-03-02 16:06:21 -05:00
parent a7bb4ec4b1
commit f26259caac
3 changed files with 36 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
util/ego/cs/cs_profit.c
View file

@ -111,6 +111,21 @@ void cs_machinit(void *vp)
choose_cset(f, &forbidden, sp_lmnem);
}
bool may_become_aar(avail_p avp)
{
/* Check whether it is desirable to treat a LAR or SAR as an
* AAR LOI/STI. This depends on the size of the array-elements.
*/
offset sz;
sz = array_elemsize(avp->av_othird);
if (sz == UNKNOWN_SIZE)
return FALSE;
if (time_space_ratio < 50)
return sz <= AR_limit;
return TRUE;
}
STATIC bool sli_no_eliminate(line_p lnp)
{
/* Return whether the SLI-instruction in lnp is part of
@ -157,8 +172,10 @@ STATIC bool gains(avail_p avp)
STATIC bool okay_lines(avail_p avp, occur_p ocp)
{
/* Check whether all lines in this occurrence can in
* principle be eliminated; no stores, messages, calls etc.
*/
register line_p lnp, next;
offset sz;
for (lnp = ocp->oc_lfirst; lnp != (line_p) 0; lnp = next) {
next = lnp != ocp->oc_llast ? lnp->l_next : (line_p) 0;
@ -171,18 +188,6 @@ STATIC bool okay_lines(avail_p avp, occur_p ocp)
return FALSE;
}
}
/* All lines in this occurrence can in principle be eliminated;
* no stores, messages, calls etc.
* We now check whether it is desirable to treat a LAR or a SAR
* as an AAR LOI/STI. This depends on the size of the array-elements.
*/
if (INSTR(ocp->oc_llast) == op_lar || INSTR(ocp->oc_llast) == op_sar) {
sz = array_elemsize(avp->av_othird);
if (sz == UNKNOWN_SIZE) return FALSE;
if (avp->av_instr == (byte) op_aar && time_space_ratio < 50) {
return sz <= AR_limit;
}
}
return TRUE;
}

6
util/ego/cs/cs_profit.h
View file

@ -7,6 +7,12 @@ void cs_machinit(void *vp); /* (FILE *f)
* Read phase-specific information from f.
*/
bool may_become_aar(avail_p avp);
/*
* Return whether a LAR/SAR may become
* an AAR LOI/STI.
*/
bool desirable(avail_p avp); /*
* Return whether it is desirable to eliminate
* the recurrences of the expression in avp.

17
util/ego/cs/cs_vnm.c
View file

@ -50,11 +50,13 @@ STATIC void put_expensive_load(bblock_p bp, line_p lnp, line_p lfirst,
STATIC void put_aar(bblock_p bp, line_p lnp, line_p lfirst, entity_p enp)
{
/* Enp points to an ENARRELEM. We do as if its address was computed. */
/* Enter the implicit AAR in a LAR or SAR, where enp points to
* the ENARRELEM, and AAR computes its address.
*/
struct avail av;
occur_p ocp;
assert(INSTR(lnp) == op_lar || INSTR(lnp) == op_sar);
assert(enp->en_kind == ENARRELEM);
av.av_instr = op_aar;
av.av_size = ps;
@ -62,9 +64,14 @@ STATIC void put_aar(bblock_p bp, line_p lnp, line_p lfirst, entity_p enp)
av.av_osecond = enp->en_index;
av.av_othird = enp->en_adesc;
ocp = newoccur(lfirst, lnp, bp);
av_enter(&av, ocp, TERNAIR_OP);
/* Before we enter an available AAR, we must check whether we
* may convert this LAR/SAR to AAR LOI/STI. This is so we
* don't LOI/STI a large or unknown size.
*/
if (may_become_aar(&av)) {
ocp = newoccur(lfirst, lnp, bp);
av_enter(&av, ocp, TERNAIR_OP);
}
}
STATIC void push_avail(avail_p avp, line_p lfirst)