| Message ID | 20131203010536.GA10039@intel.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Dec 3, 2013 at 2:05 AM, H.J. Lu <hongjiu.lu@intel.com> wrote: > emit_memset fails to adjust destination address after gen_strset, which > leads to the wrong address in aliasing info. This patch fixes it. > Tested on Linux/x86-64. OK to install? > > 2013-12-03 H.J. Lu <hongjiu.lu@intel.com> > > PR target/59363 > * config/i386/i386.c (emit_memset): Adjust destination address > after gen_strset. > > gcc/testsuite/ > > 2013-12-03 H.J. Lu <hongjiu.lu@intel.com> > > PR target/59363 > * gcc.target/i386/pr59363.c: New file. OK, but according to [1], there are other places where similar issues should be fixed. I propose to wait for Michael's analysis and eventual patch, and fix them all together. [1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59363#c21 Thanks, Uros.
Hi Uros, HJ,
I checked expand_movmem_epilogue - it seemingly doesn't have such a
problem, so the patch is ok.
We might want to add similar adjust_automodify_address_nv call to here as well:
if (TARGET_64BIT)
{
dest = change_address (destmem, DImode, destptr);
emit_insn (gen_strset (destptr, dest, value));
emit_insn (gen_strset (destptr, dest, value));
}
else
{
dest = change_address (destmem, SImode, destptr);
emit_insn (gen_strset (destptr, dest, value));
emit_insn (gen_strset (destptr, dest, value));
emit_insn (gen_strset (destptr, dest, value));
emit_insn (gen_strset (destptr, dest, value));
}
(code snippet from previous HJ's comment in bugzilla).
I think it's needed here, but I didn't manage to exploit the bug in
this code. Maybe Uros or Jan can comment whether it's needed in such
code or not.
Thanks,
Michael
On 3 December 2013 12:11, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Tue, Dec 3, 2013 at 2:05 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>
>> emit_memset fails to adjust destination address after gen_strset, which
>> leads to the wrong address in aliasing info. This patch fixes it.
>> Tested on Linux/x86-64. OK to install?
>>
>> 2013-12-03 H.J. Lu <hongjiu.lu@intel.com>
>>
>> PR target/59363
>> * config/i386/i386.c (emit_memset): Adjust destination address
>> after gen_strset.
>>
>> gcc/testsuite/
>>
>> 2013-12-03 H.J. Lu <hongjiu.lu@intel.com>
>>
>> PR target/59363
>> * gcc.target/i386/pr59363.c: New file.
>
> OK, but according to [1], there are other places where similar issues
> should be fixed. I propose to wait for Michael's analysis and eventual
> patch, and fix them all together.
>
> [1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59363#c21
>
> Thanks,
> Uros.
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index aa221df..d395a99 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -22806,6 +22806,8 @@ emit_memset (rtx destmem, rtx destptr, rtx promoted_val, if (piece_size <= GET_MODE_SIZE (word_mode)) { emit_insn (gen_strset (destptr, dst, promoted_val)); + dst = adjust_automodify_address_nv (dst, move_mode, destptr, + piece_size); continue; } diff --git a/gcc/testsuite/gcc.target/i386/pr59363.c b/gcc/testsuite/gcc.target/i386/pr59363.c new file mode 100644 index 0000000..a4e1240 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr59363.c @@ -0,0 +1,24 @@ +/* PR target/59363 */ +/* { dg-do run } */ +/* { dg-options "-O2 -mtune=amdfam10" } */ + +typedef struct { + int ctxlen; + long interhunkctxlen; + int flags; + long find_func; + void *find_func_priv; + int hunk_func; +} xdemitconf_t; + +__attribute__((noinline)) +int xdi_diff(xdemitconf_t *xecfg) { + if (xecfg->hunk_func == 0) + __builtin_abort(); + return 0; +} +int main() { + xdemitconf_t xecfg = {0}; + xecfg.hunk_func = 1; + return xdi_diff(&xecfg); +}