| Message ID | 20201203085049.GA3788@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | i386: Fix up ix86_md_asm_adjust for TImode [PR98086] | expand |
On Thu, Dec 3, 2020 at 9:50 AM Jakub Jelinek <jakub@redhat.com> wrote: > > Hi! > > ix86_md_asm_adjust right above this code uses: > machine_mode dest_mode = GET_MODE (dest); > if (!SCALAR_INT_MODE_P (dest_mode)) > { > error ("invalid type for %<asm%> flag output"); > continue; > } > but then assumes that dest_mode can be only [QHSD]Imode and nothing else. > > The following patch handles TImode and hypothetically even wider modes > by handling it like DImode for 32-bit. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2020-12-03 Jakub Jelinek <jakub@redhat.com> > > PR target/98086 > * config/i386/i386.c (ix86_md_asm_adjust): Use dest_mode DImode > or SImode for modes wider than DImode. If dest_mode isn't SImode > or GET_MODE (dest) isn't DImode, use convert_to_mode. The whole part should be simply rewritten to: --cut here-- if (dest_mode == QImode) emit_insn (gen_rtx_SET (dest, x)); else { rtx reg = gen_reg_rtx (QImode); emit_insn (gen_rtx_SET (reg, x)); reg = convert_to_mode (GET_MODE (dest), reg, 1); emit_move_insn (dest, reg); } --cut here-- Uros.
On Thu, Dec 3, 2020 at 11:39 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > ix86_md_asm_adjust right above this code uses: > > machine_mode dest_mode = GET_MODE (dest); > > if (!SCALAR_INT_MODE_P (dest_mode)) > > { > > error ("invalid type for %<asm%> flag output"); > > continue; > > } > > but then assumes that dest_mode can be only [QHSD]Imode and nothing else. > > > > The following patch handles TImode and hypothetically even wider modes > > by handling it like DImode for 32-bit. > > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > > > 2020-12-03 Jakub Jelinek <jakub@redhat.com> > > > > PR target/98086 > > * config/i386/i386.c (ix86_md_asm_adjust): Use dest_mode DImode > > or SImode for modes wider than DImode. If dest_mode isn't SImode > > or GET_MODE (dest) isn't DImode, use convert_to_mode. > > The whole part should be simply rewritten to: > > --cut here-- > if (dest_mode == QImode) > emit_insn (gen_rtx_SET (dest, x)); > else > { > rtx reg = gen_reg_rtx (QImode); > emit_insn (gen_rtx_SET (reg, x)); > > reg = convert_to_mode (GET_MODE (dest), reg, 1); > emit_move_insn (dest, reg); > } > --cut here-- Please let me fix this PR. Uros.
On Thu, Dec 03, 2020 at 12:49:49PM +0100, Uros Bizjak wrote: > On Thu, Dec 3, 2020 at 11:39 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > ix86_md_asm_adjust right above this code uses: > > > machine_mode dest_mode = GET_MODE (dest); > > > if (!SCALAR_INT_MODE_P (dest_mode)) > > > { > > > error ("invalid type for %<asm%> flag output"); > > > continue; > > > } > > > but then assumes that dest_mode can be only [QHSD]Imode and nothing else. > > > > > > The following patch handles TImode and hypothetically even wider modes > > > by handling it like DImode for 32-bit. > > > > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > > > > > 2020-12-03 Jakub Jelinek <jakub@redhat.com> > > > > > > PR target/98086 > > > * config/i386/i386.c (ix86_md_asm_adjust): Use dest_mode DImode > > > or SImode for modes wider than DImode. If dest_mode isn't SImode > > > or GET_MODE (dest) isn't DImode, use convert_to_mode. > > > > The whole part should be simply rewritten to: > > > > --cut here-- > > if (dest_mode == QImode) > > emit_insn (gen_rtx_SET (dest, x)); > > else > > { > > rtx reg = gen_reg_rtx (QImode); > > emit_insn (gen_rtx_SET (reg, x)); > > > > reg = convert_to_mode (GET_MODE (dest), reg, 1); > > emit_move_insn (dest, reg); > > } > > --cut here-- > > Please let me fix this PR. Sorry, I've been looking at the glibc miscompilation in the meantime. But if you want to take this over, please go ahead, it is all yours, there are hundreds of other PRs for me to choose from ;) Jakub
--- gcc/config/i386/i386.c.jj 2020-11-18 09:40:09.498664422 +0100 +++ gcc/config/i386/i386.c 2020-12-02 15:33:26.224184113 +0100 @@ -21508,6 +21508,8 @@ ix86_md_asm_adjust (vec<rtx> &outputs, v continue; } + if (GET_MODE_BITSIZE (dest_mode) > GET_MODE_BITSIZE (DImode)) + dest_mode = DImode; if (dest_mode == DImode && !TARGET_64BIT) dest_mode = SImode; @@ -21534,10 +21536,16 @@ ix86_md_asm_adjust (vec<rtx> &outputs, v if (dest_mode != GET_MODE (dest)) { - rtx tmp = gen_reg_rtx (SImode); + rtx tmp = gen_reg_rtx (dest_mode); emit_insn (gen_rtx_SET (tmp, x)); - emit_insn (gen_zero_extendsidi2 (dest, tmp)); + if (dest_mode == SImode && GET_MODE (dest) == DImode) + emit_insn (gen_zero_extendsidi2 (dest, tmp)); + else + { + tmp = convert_to_mode (GET_MODE (dest), tmp, 1); + emit_insn (gen_rtx_SET (dest, tmp)); + } } else emit_insn (gen_rtx_SET (dest, x)); --- gcc/testsuite/gcc.target/i386/pr98086.c.jj 2020-12-02 15:36:40.858951118 +0100 +++ gcc/testsuite/gcc.target/i386/pr98086.c 2020-12-02 15:37:27.305406401 +0100 @@ -0,0 +1,10 @@ +/* PR target/98086 */ +/* { dg-do compile { target int128 } } */ + +__int128_t x; + +void +foo (void) +{ + __asm ("" : "=@ccc" (x)); +}
Hi! ix86_md_asm_adjust right above this code uses: machine_mode dest_mode = GET_MODE (dest); if (!SCALAR_INT_MODE_P (dest_mode)) { error ("invalid type for %<asm%> flag output"); continue; } but then assumes that dest_mode can be only [QHSD]Imode and nothing else. The following patch handles TImode and hypothetically even wider modes by handling it like DImode for 32-bit. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2020-12-03 Jakub Jelinek <jakub@redhat.com> PR target/98086 * config/i386/i386.c (ix86_md_asm_adjust): Use dest_mode DImode or SImode for modes wider than DImode. If dest_mode isn't SImode or GET_MODE (dest) isn't DImode, use convert_to_mode. * gcc.target/i386/pr98086.c: New test. Jakub