| Message ID | BANLkTik-7forNtBN9i27aTeN+NuCtVyBew@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sun, Jun 12, 2011 at 7:33 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Sun, Jun 12, 2011 at 7:00 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Sun, Jun 12, 2011 at 6:50 AM, Richard Guenther >> <richard.guenther@gmail.com> wrote: >>> On Sun, Jun 12, 2011 at 3:18 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>> On Sun, Jun 12, 2011 at 3:48 AM, Richard Guenther >>>> <richard.guenther@gmail.com> wrote: >>>>> On Sat, Jun 11, 2011 at 5:09 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >>>>>> Hi, >>>>>> >>>>>> expand_builtin_strlen has >>>>>> >>>>>> src_reg = gen_reg_rtx (Pmode); >>>>>> ... >>>>>> pat = expand_expr (src, src_reg, ptr_mode, EXPAND_NORMAL); >>>>>> if (pat != src_reg) >>>>>> emit_move_insn (src_reg, pat); >>>>>> >>>>>> But src_reg may be in ptr_mode, wich may not be the same as Pmode. >>>>>> This patch checks it. OK for trunk? >>>>>> >>>>>> Thanks. >>>>>> >>>>>> >>>>>> H.J. >>>>>> --- >>>>>> 2011-06-11 H.J. Lu <hongjiu.lu@intel.com> >>>>>> >>>>>> PR middle-end/47364 >>>>>> * builtins.c (expand_builtin_strlen): Properly handle target >>>>>> not in Pmode. >>>>>> >>>>>> diff --git a/gcc/builtins.c b/gcc/builtins.c >>>>>> index 7b24a0c..4e2cf31 100644 >>>>>> --- a/gcc/builtins.c >>>>>> +++ b/gcc/builtins.c >>>>>> @@ -2941,7 +2941,11 @@ expand_builtin_strlen (tree exp, rtx target, >>>>>> start_sequence (); >>>>>> pat = expand_expr (src, src_reg, ptr_mode, EXPAND_NORMAL); >>>>>> if (pat != src_reg) >>>>>> - emit_move_insn (src_reg, pat); >>>>>> + { >>>>>> + if (GET_MODE (pat) != Pmode) >>>>>> + pat = convert_to_mode (Pmode, pat, 1); >>>>> >>>>> Shouldn't this be POINTERS_EXTEND_UNSIGNED instead of 1? >>>>> >>>>>> + emit_move_insn (src_reg, pat); >>>>> >>>>> Why not use convert_move unconditionally? >>>>> >>>>> Or, why not expand src in Pmode from the start? After all, src_reg is >>>>> created as Pmode reg. >>>>> >>>> >>>> This patch works for my testcase. OK for trunk? >>> >>> Ok if it passes bootstrap & regtest on a ptr_mode != Pmode target. >>> >> >> Only the following targets expand strlen: >> >> avr/avr.md:(define_expand "strlenhi" >> avr/avr.md:(define_insn "*strlenhi" >> i386/i386.md:(define_expand "strlen<mode>" >> i386/i386.md: if (ix86_expand_strlen (operands[0], operands[1], >> operands[2], operands[3])) >> i386/i386.md:(define_expand "strlenqi_1" >> i386/i386.md:(define_insn "*strlenqi_1" >> rs6000/rs6000.md:(define_expand "strlensi" >> s390/s390.md:; strlenM instruction pattern(s). >> s390/s390.md:(define_expand "strlen<mode>" >> s390/s390.md:(define_insn "*strlen<mode>" >> >> None of them, except for my x32 port, are ptr_mode != Pmode targets. >> I will bootstrap and test it on my x32 branch. >> > > It doesn't work on x32. I got > > /export/gnu/import/git/gcc-x32/libssp/gets-chk.c:74:14: internal > compiler error: in emit_move_insn, at expr.c:3319 > Please submit a full bug report, > with preprocessed source if appropriate. > See <http://gcc.gnu.org/bugs.html> for instructions. > > How about this patch? > > Thanks. No regressions on x32 branch. OK for trunk? Thanks. > -- > H.J. > --- > 2011-06-12 H.J. Lu <hongjiu.lu@intel.com> > > PR middle-end/47364 > * builtins.c (expand_builtin_strlen): Properly handle target > not in Pmode. > > diff --git a/gcc/builtins.c b/gcc/builtins.c > index 7b24a0c..a2f175d 100644 > --- a/gcc/builtins.c > +++ b/gcc/builtins.c > @@ -2941,7 +2941,14 @@ expand_builtin_strlen (tree exp, rtx target, > start_sequence (); > pat = expand_expr (src, src_reg, ptr_mode, EXPAND_NORMAL); > if (pat != src_reg) > - emit_move_insn (src_reg, pat); > + { > +#ifdef POINTERS_EXTEND_UNSIGNED > + if (GET_MODE (pat) != Pmode) > + pat = convert_to_mode (Pmode, pat, > + POINTERS_EXTEND_UNSIGNED); > +#endif > + emit_move_insn (src_reg, pat); > + } > pat = get_insns (); > end_sequence (); >
On Sun, Jun 12, 2011 at 6:28 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Sun, Jun 12, 2011 at 7:33 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Sun, Jun 12, 2011 at 7:00 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> On Sun, Jun 12, 2011 at 6:50 AM, Richard Guenther >>> <richard.guenther@gmail.com> wrote: >>>> On Sun, Jun 12, 2011 at 3:18 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>>> On Sun, Jun 12, 2011 at 3:48 AM, Richard Guenther >>>>> <richard.guenther@gmail.com> wrote: >>>>>> On Sat, Jun 11, 2011 at 5:09 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >>>>>>> Hi, >>>>>>> >>>>>>> expand_builtin_strlen has >>>>>>> >>>>>>> src_reg = gen_reg_rtx (Pmode); >>>>>>> ... >>>>>>> pat = expand_expr (src, src_reg, ptr_mode, EXPAND_NORMAL); >>>>>>> if (pat != src_reg) >>>>>>> emit_move_insn (src_reg, pat); >>>>>>> >>>>>>> But src_reg may be in ptr_mode, wich may not be the same as Pmode. >>>>>>> This patch checks it. OK for trunk? >>>>>>> >>>>>>> Thanks. >>>>>>> >>>>>>> >>>>>>> H.J. >>>>>>> --- >>>>>>> 2011-06-11 H.J. Lu <hongjiu.lu@intel.com> >>>>>>> >>>>>>> PR middle-end/47364 >>>>>>> * builtins.c (expand_builtin_strlen): Properly handle target >>>>>>> not in Pmode. >>>>>>> >>>>>>> diff --git a/gcc/builtins.c b/gcc/builtins.c >>>>>>> index 7b24a0c..4e2cf31 100644 >>>>>>> --- a/gcc/builtins.c >>>>>>> +++ b/gcc/builtins.c >>>>>>> @@ -2941,7 +2941,11 @@ expand_builtin_strlen (tree exp, rtx target, >>>>>>> start_sequence (); >>>>>>> pat = expand_expr (src, src_reg, ptr_mode, EXPAND_NORMAL); >>>>>>> if (pat != src_reg) >>>>>>> - emit_move_insn (src_reg, pat); >>>>>>> + { >>>>>>> + if (GET_MODE (pat) != Pmode) >>>>>>> + pat = convert_to_mode (Pmode, pat, 1); >>>>>> >>>>>> Shouldn't this be POINTERS_EXTEND_UNSIGNED instead of 1? >>>>>> >>>>>>> + emit_move_insn (src_reg, pat); >>>>>> >>>>>> Why not use convert_move unconditionally? >>>>>> >>>>>> Or, why not expand src in Pmode from the start? After all, src_reg is >>>>>> created as Pmode reg. >>>>>> >>>>> >>>>> This patch works for my testcase. OK for trunk? >>>> >>>> Ok if it passes bootstrap & regtest on a ptr_mode != Pmode target. >>>> >>> >>> Only the following targets expand strlen: >>> >>> avr/avr.md:(define_expand "strlenhi" >>> avr/avr.md:(define_insn "*strlenhi" >>> i386/i386.md:(define_expand "strlen<mode>" >>> i386/i386.md: if (ix86_expand_strlen (operands[0], operands[1], >>> operands[2], operands[3])) >>> i386/i386.md:(define_expand "strlenqi_1" >>> i386/i386.md:(define_insn "*strlenqi_1" >>> rs6000/rs6000.md:(define_expand "strlensi" >>> s390/s390.md:; strlenM instruction pattern(s). >>> s390/s390.md:(define_expand "strlen<mode>" >>> s390/s390.md:(define_insn "*strlen<mode>" >>> >>> None of them, except for my x32 port, are ptr_mode != Pmode targets. >>> I will bootstrap and test it on my x32 branch. >>> >> >> It doesn't work on x32. I got >> >> /export/gnu/import/git/gcc-x32/libssp/gets-chk.c:74:14: internal >> compiler error: in emit_move_insn, at expr.c:3319 >> Please submit a full bug report, >> with preprocessed source if appropriate. >> See <http://gcc.gnu.org/bugs.html> for instructions. >> >> How about this patch? >> >> Thanks. > > No regressions on x32 branch. OK for trunk? Does it work with also doing the expansion to Pmode in the first place? If so, ok with that change. Thanks, Richard. > Thanks. > >> -- >> H.J. >> --- >> 2011-06-12 H.J. Lu <hongjiu.lu@intel.com> >> >> PR middle-end/47364 >> * builtins.c (expand_builtin_strlen): Properly handle target >> not in Pmode. >> >> diff --git a/gcc/builtins.c b/gcc/builtins.c >> index 7b24a0c..a2f175d 100644 >> --- a/gcc/builtins.c >> +++ b/gcc/builtins.c >> @@ -2941,7 +2941,14 @@ expand_builtin_strlen (tree exp, rtx target, >> start_sequence (); >> pat = expand_expr (src, src_reg, ptr_mode, EXPAND_NORMAL); >> if (pat != src_reg) >> - emit_move_insn (src_reg, pat); >> + { >> +#ifdef POINTERS_EXTEND_UNSIGNED >> + if (GET_MODE (pat) != Pmode) >> + pat = convert_to_mode (Pmode, pat, >> + POINTERS_EXTEND_UNSIGNED); >> +#endif >> + emit_move_insn (src_reg, pat); >> + } >> pat = get_insns (); >> end_sequence (); >> > > > > -- > H.J. >
diff --git a/gcc/builtins.c b/gcc/builtins.c index 7b24a0c..a2f175d 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -2941,7 +2941,14 @@ expand_builtin_strlen (tree exp, rtx target, start_sequence (); pat = expand_expr (src, src_reg, ptr_mode, EXPAND_NORMAL); if (pat != src_reg) - emit_move_insn (src_reg, pat); + { +#ifdef POINTERS_EXTEND_UNSIGNED + if (GET_MODE (pat) != Pmode) + pat = convert_to_mode (Pmode, pat, + POINTERS_EXTEND_UNSIGNED); +#endif + emit_move_insn (src_reg, pat); + } pat = get_insns (); end_sequence ();