[Bug middle-end/78355] New: LRA generates unaligned accesses when SLOW_UNALIGNED_ACCESS is 1

[pipcet at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78355

            Bug ID: 78355
           Summary: LRA generates unaligned accesses when
                    SLOW_UNALIGNED_ACCESS is 1
           Product: gcc
           Version: 7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pipcet at gmail dot com
  Target Milestone: ---

Created attachment 40041
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=40041&action=edit
patch to illustrate proposed fix

(I hope I've selected the right component).

I'm working on a gcc (+binutils,glibc) backend for asm.js at
https://github.com/pipcet/asmjs. Things are working fairly well, but I
think I found a bug in the LRA handling of (subreg:SI (mem:HI ...))
expressions when STRICT_ALIGNMENT is true.

Summary: applying the attached patch will prevent the creation of
unaligned memory accesses on targets that don't support them, but it's
probably not the best fix.

I'm using LRA, as is required for new ports.

asm.js truncates unaligned accesses. I read the documentation as
suggesting I #define STRICT_ALIGNMENT 1 in this case, and #define
SLOW_UNALIGNED_ACCESS(x,y) 1 as well, which is what defaults.h would
have done had I not provided a definition.

In compiling the following code:

short int x[] = {
  1, 2, 3, 4, 5, -1, -2,
};
int f(int index)
{
  return x[index];
}

I see that the reload (LRA) pass turns

(insn 10 9 11 2 (set (reg:SI 45)
        (ashift:SI (subreg:SI (mem:HI (plus:SI (reg:SI 47)
                        (symbol_ref:SI ("y") [flags 0x2]  <var_decl
0x7ffff7fec510 y>)) [1 y S2 A16]) 0)
            (const_int 16 [0x10]))) "sign-extend.c":16 9 {*assignsi_binop}
     (expr_list:REG_DEAD (reg:SI 47)
        (nil)))

into

(insn 10 9 11 2 (set (reg:SI 8 r0 [45])
        (ashift:SI (mem:SI (plus:SI (reg:SI 8 r0 [47])
                    (symbol_ref:SI ("y") [flags 0x2]  <var_decl 0x7ffff7fec510
y>)) [1 y S4 A16])
            (const_int 16 [0x10]))) "sign-extend.c":16 9 {*assignsi_binop}
     (nil))


The former is correct; the latter is not, because the access might be
unaligned and silently access the wrong data.

The problem is this code in simplify_operand_subreg (lra-constraints.c):

          if (!SLOW_UNALIGNED_ACCESS (mode, MEM_ALIGN (reg))
              || SLOW_UNALIGNED_ACCESS (innermode, MEM_ALIGN (reg))
              || MEM_ALIGN (reg) >= GET_MODE_ALIGNMENT (mode))
            return true;

          ... reload the mem in innermode

the second SLOW_UNALIGNED_ACCESS() expression is true (as it's defined
to be so in all cases), so we never reload the mem expression in the
right mode.

Commenting out the second line fixes the problem.

I think the problem here is that SLOW_UNALIGNED_ACCESS is sloppily
defined by my port, and defaults.h, to be a constant, but it is then
used with naturally-aligned data, access to which is never slow, and
expected to return false. So it would probably be best to replace the
condition by

          if (!SLOW_UNALIGNED_ACCESS (mode, MEM_ALIGN (reg))
              || (MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (innermode)
                  && SLOW_UNALIGNED_ACCESS (innermode, MEM_ALIGN (reg)))
              || MEM_ALIGN (reg) >= GET_MODE_ALIGNMENT (mode))
            return true;

Or is there a better way? I hope I'm not totally wrong in my analysis.