This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Dec alias patch causing regressions
- To: kenner at vlsi1 dot ultra dot nyu dot edu
- Subject: Dec alias patch causing regressions
- From: Jeffrey A Law <law at redhat dot com>
- Date: Sun, 11 Mar 2001 21:25:25 -0700
- Cc: gcc at gcc dot gnu dot org
- Reply-To: law at redhat dot com
This patch from December is causing regressions on multiple platforms
(ia32 and PA) on the gcc-3.0 release branch relative to gcc-2.95.2.
Specifically bf64-1.c and 991118-1.c are failing at -O3 because we're
incorrectly deleting stores into the stack frame.
Basically we have multiple stores into the same stack slots using different
alias sets. That's bad. Doubly so since it was created by the compiler,
not by dubious user code.
2000-12-30 Richard Kenner <kenner@vlsi1.ultra.nyu.edu>
* calls.c (calls_function_1, case CONSTRUCTOR): New case.
* alias.c (alias_sets_conflict_p): New function.
(mems_in_disjoint_alias_sets_p): Use it.
(readonly_fields_p): Moved from expr.c; check for record type.
(objects_must_conflict_p): New function.
* calls.c (expand_call): Use assign_temp as much as possible, use
readonly variant if assigned once, and don't set memory attributes.
(emit_library_call_value_1, store_one_arg): Likewise.
* integrate.c (expand_inline_function): Likewise.
* stmt.c (expand_asm_operands, expand_return): Likewise.
* expr.c (copy_blkmode_from_reg, store_constructor): Likewise.
(store_field, save_noncopied_parts, expand_expr): Likewise.
(expand_expr_unaligned): Likewise.
(readonly_fields_p): Moved to alias.c.
(safe_from_p): Rework handling of SAVE_EXPR.
MEMs ony conflict if alias sets conflict; likewise for INDIRECT_REF.
* function.c (struct temp_slot): Delete field ALIAS_SET; add TYPE.
(assign_stack_for_temp): Use objects_must_confict_p.
Set all memory attributes from type, if specified.
(mark_temp_slot): Mark TYPE field.
Consider the attached slightly simplified version of bf-64.c on the PA.
In the .cfg dump we have:
(insn 16 15 17 (set (mem/s:DI (plus:SI (reg/f:SI 3 %r3)
(const_int 16 [0x10])) 4)
(reg:DI 97)) 119 {*pa.md:3094} (nil)
(nil))
[ ... ]
(insn/i 21 20 22 (set (reg:SI 100)
(mem/s:SI (plus:SI (reg/f:SI 3 %r3)
(const_int 16 [0x10])) 4)) 68 {*pa.md:2088} (nil)
(nil))
[ ... ]
(insn/i 46 45 47 (set (mem/s:SI (plus:SI (reg/f:SI 3 %r3)
(const_int 16 [0x10])) 4)
(reg:SI 114)) 68 {*pa.md:2088} (nil)
(nil))
[ ... ]
(insn 87 86 97 (set (mem/s:DI (plus:SI (reg/f:SI 3 %r3)
(const_int 16 [0x10])) 5)
(reg:DI 128)) 119 {*pa.md:3094} (nil)
(nil))
Note carefully that the alias set on insn 87 is different from the alias
set on the other 3 insns.
During dead store elimination we correctly remove insn 46 (there's nothing
which ever reads the value we stored). However, we also remove insn 16
which is incorrect as the value we store is read by insn 21. [ this happens
because we consider the memory references in insn 21 and 87 as not
aliasing each other because they have different alias sets.
I'm somewhat disturbed that you broke this 3 months ago on multiple platforms,
but haven't tried to fix it as far as I can tell. Given this is a regression
on the release branch relative to 2.95.2 that affects multiple platforms I
hope you'll give it immediate attention.
struct tmp
{
long long int pad : 12;
long long int field : 52;
};
struct tmp2
{
long long int field : 52;
long long int pad : 12;
};
static struct tmp
sub (struct tmp tmp)
{
tmp.field ^= 0x0008765412345678LL;
return tmp;
}
static void
sub2 (struct tmp2 tmp2)
{
}
struct tmp tmp = {0x123, 0x123456789ABCDLL};
struct tmp2 tmp2;
main()
{
tmp = sub (tmp);
sub2 (tmp2);
if (tmp.pad != 0x123)
abort ();
exit (0);
}