Message ID | 340fcec1-eb5e-e0f6-e8d1-ea819d4fd165@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | rs6000: Handle unresolved overloaded builtin [PR105485] | expand |
Hi, Gentle ping https://gcc.gnu.org/pipermail/gcc-patches/2022-May/594699.html BR, Kewen on 2022/5/13 13:29, Kewen.Lin via Gcc-patches wrote: > Hi, > > PR105485 exposes that new builtin function framework doesn't handle > unresolved overloaded builtin function well. With new builtin > function support, we don't have builtin info for any overloaded > rs6000_gen_builtins enum, since they are expected to be resolved to > one specific instance. So when function rs6000_gimple_fold_builtin > faces one unresolved overloaded builtin, the access for builtin info > becomes out of bound and gets ICE then. > > We should not try to fold one unresolved overloaded builtin there > and as the previous support we should emit one error message during > expansion phase like "unresolved overload for builtin ...". > > Bootstrapped and regtested on powerpc64-linux-gnu P8 and > powerpc64le-linux-gnu P9 and P10. > > Is it ok for trunk? > > BR, > Kewen > ----- > PR target/105485 > > gcc/ChangeLog: > > * config/rs6000/rs6000-builtin.cc (rs6000_gimple_fold_builtin): Add > the handling for unresolved overloaded builtin function. > (rs6000_expand_builtin): Likewise. > > gcc/testsuite/ChangeLog: > > * g++.target/powerpc/pr105485.C: New test. > > --- > gcc/config/rs6000/rs6000-builtin.cc | 13 +++++++++++++ > gcc/testsuite/g++.target/powerpc/pr105485.C | 9 +++++++++ > 2 files changed, 22 insertions(+) > create mode 100644 gcc/testsuite/g++.target/powerpc/pr105485.C > > diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc > index e925ba9fad9..e102305c90c 100644 > --- a/gcc/config/rs6000/rs6000-builtin.cc > +++ b/gcc/config/rs6000/rs6000-builtin.cc > @@ -1294,6 +1294,11 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi) > enum tree_code bcode; > gimple *g; > > + /* For an unresolved overloaded builtin, return early here since there > + is no builtin info for it and we are unable to fold it. */ > + if (fn_code > RS6000_OVLD_NONE) > + return false; > + > size_t uns_fncode = (size_t) fn_code; > enum insn_code icode = rs6000_builtin_info[uns_fncode].icode; > const char *fn_name1 = rs6000_builtin_info[uns_fncode].bifname; > @@ -3295,6 +3300,14 @@ rs6000_expand_builtin (tree exp, rtx target, rtx /* subtarget */, > tree fndecl = TREE_OPERAND (CALL_EXPR_FN (exp), 0); > enum rs6000_gen_builtins fcode > = (enum rs6000_gen_builtins) DECL_MD_FUNCTION_CODE (fndecl); > + > + /* Emit error message if it's an unresolved overloaded builtin. */ > + if (fcode > RS6000_OVLD_NONE) > + { > + error ("unresolved overload for builtin %qF", fndecl); > + return const0_rtx; > + } > + > size_t uns_fcode = (size_t)fcode; > enum insn_code icode = rs6000_builtin_info[uns_fcode].icode; > > diff --git a/gcc/testsuite/g++.target/powerpc/pr105485.C b/gcc/testsuite/g++.target/powerpc/pr105485.C > new file mode 100644 > index 00000000000..a3b8290df8c > --- /dev/null > +++ b/gcc/testsuite/g++.target/powerpc/pr105485.C > @@ -0,0 +1,9 @@ > +/* It's to verify no ICE here, ignore error/warning messages since > + they are not test points here. */ > +/* { dg-excess-errors "pr105485" } */ > + > +template <class> void __builtin_vec_vslv(); > +typedef __attribute__((altivec(vector__))) char T; > +T b (T c, T d) { > + return __builtin_vec_vslv(c, d); > +}
Hi, Gentle ping https://gcc.gnu.org/pipermail/gcc-patches/2022-May/594699.html BR, Kewen > on 2022/5/13 13:29, Kewen.Lin via Gcc-patches wrote: >> Hi, >> >> PR105485 exposes that new builtin function framework doesn't handle >> unresolved overloaded builtin function well. With new builtin >> function support, we don't have builtin info for any overloaded >> rs6000_gen_builtins enum, since they are expected to be resolved to >> one specific instance. So when function rs6000_gimple_fold_builtin >> faces one unresolved overloaded builtin, the access for builtin info >> becomes out of bound and gets ICE then. >> >> We should not try to fold one unresolved overloaded builtin there >> and as the previous support we should emit one error message during >> expansion phase like "unresolved overload for builtin ...". >> >> Bootstrapped and regtested on powerpc64-linux-gnu P8 and >> powerpc64le-linux-gnu P9 and P10. >> >> Is it ok for trunk? >> >> BR, >> Kewen >> ----- >> PR target/105485 >> >> gcc/ChangeLog: >> >> * config/rs6000/rs6000-builtin.cc (rs6000_gimple_fold_builtin): Add >> the handling for unresolved overloaded builtin function. >> (rs6000_expand_builtin): Likewise. >> >> gcc/testsuite/ChangeLog: >> >> * g++.target/powerpc/pr105485.C: New test. >> >> --- >> gcc/config/rs6000/rs6000-builtin.cc | 13 +++++++++++++ >> gcc/testsuite/g++.target/powerpc/pr105485.C | 9 +++++++++ >> 2 files changed, 22 insertions(+) >> create mode 100644 gcc/testsuite/g++.target/powerpc/pr105485.C >> >> diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc >> index e925ba9fad9..e102305c90c 100644 >> --- a/gcc/config/rs6000/rs6000-builtin.cc >> +++ b/gcc/config/rs6000/rs6000-builtin.cc >> @@ -1294,6 +1294,11 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi) >> enum tree_code bcode; >> gimple *g; >> >> + /* For an unresolved overloaded builtin, return early here since there >> + is no builtin info for it and we are unable to fold it. */ >> + if (fn_code > RS6000_OVLD_NONE) >> + return false; >> + >> size_t uns_fncode = (size_t) fn_code; >> enum insn_code icode = rs6000_builtin_info[uns_fncode].icode; >> const char *fn_name1 = rs6000_builtin_info[uns_fncode].bifname; >> @@ -3295,6 +3300,14 @@ rs6000_expand_builtin (tree exp, rtx target, rtx /* subtarget */, >> tree fndecl = TREE_OPERAND (CALL_EXPR_FN (exp), 0); >> enum rs6000_gen_builtins fcode >> = (enum rs6000_gen_builtins) DECL_MD_FUNCTION_CODE (fndecl); >> + >> + /* Emit error message if it's an unresolved overloaded builtin. */ >> + if (fcode > RS6000_OVLD_NONE) >> + { >> + error ("unresolved overload for builtin %qF", fndecl); >> + return const0_rtx; >> + } >> + >> size_t uns_fcode = (size_t)fcode; >> enum insn_code icode = rs6000_builtin_info[uns_fcode].icode; >> >> diff --git a/gcc/testsuite/g++.target/powerpc/pr105485.C b/gcc/testsuite/g++.target/powerpc/pr105485.C >> new file mode 100644 >> index 00000000000..a3b8290df8c >> --- /dev/null >> +++ b/gcc/testsuite/g++.target/powerpc/pr105485.C >> @@ -0,0 +1,9 @@ >> +/* It's to verify no ICE here, ignore error/warning messages since >> + they are not test points here. */ >> +/* { dg-excess-errors "pr105485" } */ >> + >> +template <class> void __builtin_vec_vslv(); >> +typedef __attribute__((altivec(vector__))) char T; >> +T b (T c, T d) { >> + return __builtin_vec_vslv(c, d); >> +}
Hi, Gentle ping https://gcc.gnu.org/pipermail/gcc-patches/2022-May/594699.html BR, Kewen > >> on 2022/5/13 13:29, Kewen.Lin via Gcc-patches wrote: >>> Hi, >>> >>> PR105485 exposes that new builtin function framework doesn't handle >>> unresolved overloaded builtin function well. With new builtin >>> function support, we don't have builtin info for any overloaded >>> rs6000_gen_builtins enum, since they are expected to be resolved to >>> one specific instance. So when function rs6000_gimple_fold_builtin >>> faces one unresolved overloaded builtin, the access for builtin info >>> becomes out of bound and gets ICE then. >>> >>> We should not try to fold one unresolved overloaded builtin there >>> and as the previous support we should emit one error message during >>> expansion phase like "unresolved overload for builtin ...". >>> >>> Bootstrapped and regtested on powerpc64-linux-gnu P8 and >>> powerpc64le-linux-gnu P9 and P10. >>> >>> Is it ok for trunk? >>> >>> BR, >>> Kewen >>> ----- >>> PR target/105485 >>> >>> gcc/ChangeLog: >>> >>> * config/rs6000/rs6000-builtin.cc (rs6000_gimple_fold_builtin): Add >>> the handling for unresolved overloaded builtin function. >>> (rs6000_expand_builtin): Likewise. >>> >>> gcc/testsuite/ChangeLog: >>> >>> * g++.target/powerpc/pr105485.C: New test. >>> >>> --- >>> gcc/config/rs6000/rs6000-builtin.cc | 13 +++++++++++++ >>> gcc/testsuite/g++.target/powerpc/pr105485.C | 9 +++++++++ >>> 2 files changed, 22 insertions(+) >>> create mode 100644 gcc/testsuite/g++.target/powerpc/pr105485.C >>> >>> diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc >>> index e925ba9fad9..e102305c90c 100644 >>> --- a/gcc/config/rs6000/rs6000-builtin.cc >>> +++ b/gcc/config/rs6000/rs6000-builtin.cc >>> @@ -1294,6 +1294,11 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi) >>> enum tree_code bcode; >>> gimple *g; >>> >>> + /* For an unresolved overloaded builtin, return early here since there >>> + is no builtin info for it and we are unable to fold it. */ >>> + if (fn_code > RS6000_OVLD_NONE) >>> + return false; >>> + >>> size_t uns_fncode = (size_t) fn_code; >>> enum insn_code icode = rs6000_builtin_info[uns_fncode].icode; >>> const char *fn_name1 = rs6000_builtin_info[uns_fncode].bifname; >>> @@ -3295,6 +3300,14 @@ rs6000_expand_builtin (tree exp, rtx target, rtx /* subtarget */, >>> tree fndecl = TREE_OPERAND (CALL_EXPR_FN (exp), 0); >>> enum rs6000_gen_builtins fcode >>> = (enum rs6000_gen_builtins) DECL_MD_FUNCTION_CODE (fndecl); >>> + >>> + /* Emit error message if it's an unresolved overloaded builtin. */ >>> + if (fcode > RS6000_OVLD_NONE) >>> + { >>> + error ("unresolved overload for builtin %qF", fndecl); >>> + return const0_rtx; >>> + } >>> + >>> size_t uns_fcode = (size_t)fcode; >>> enum insn_code icode = rs6000_builtin_info[uns_fcode].icode; >>> >>> diff --git a/gcc/testsuite/g++.target/powerpc/pr105485.C b/gcc/testsuite/g++.target/powerpc/pr105485.C >>> new file mode 100644 >>> index 00000000000..a3b8290df8c >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.target/powerpc/pr105485.C >>> @@ -0,0 +1,9 @@ >>> +/* It's to verify no ICE here, ignore error/warning messages since >>> + they are not test points here. */ >>> +/* { dg-excess-errors "pr105485" } */ >>> + >>> +template <class> void __builtin_vec_vslv(); >>> +typedef __attribute__((altivec(vector__))) char T; >>> +T b (T c, T d) { >>> + return __builtin_vec_vslv(c, d); >>> +}
Hi, Gentle ping https://gcc.gnu.org/pipermail/gcc-patches/2022-May/594699.html BR, Kewen >> >>> on 2022/5/13 13:29, Kewen.Lin via Gcc-patches wrote: >>>> Hi, >>>> >>>> PR105485 exposes that new builtin function framework doesn't handle >>>> unresolved overloaded builtin function well. With new builtin >>>> function support, we don't have builtin info for any overloaded >>>> rs6000_gen_builtins enum, since they are expected to be resolved to >>>> one specific instance. So when function rs6000_gimple_fold_builtin >>>> faces one unresolved overloaded builtin, the access for builtin info >>>> becomes out of bound and gets ICE then. >>>> >>>> We should not try to fold one unresolved overloaded builtin there >>>> and as the previous support we should emit one error message during >>>> expansion phase like "unresolved overload for builtin ...". >>>> >>>> Bootstrapped and regtested on powerpc64-linux-gnu P8 and >>>> powerpc64le-linux-gnu P9 and P10. >>>> >>>> Is it ok for trunk? >>>> >>>> BR, >>>> Kewen >>>> ----- >>>> PR target/105485 >>>> >>>> gcc/ChangeLog: >>>> >>>> * config/rs6000/rs6000-builtin.cc (rs6000_gimple_fold_builtin): Add >>>> the handling for unresolved overloaded builtin function. >>>> (rs6000_expand_builtin): Likewise. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> * g++.target/powerpc/pr105485.C: New test. >>>> >>>> --- >>>> gcc/config/rs6000/rs6000-builtin.cc | 13 +++++++++++++ >>>> gcc/testsuite/g++.target/powerpc/pr105485.C | 9 +++++++++ >>>> 2 files changed, 22 insertions(+) >>>> create mode 100644 gcc/testsuite/g++.target/powerpc/pr105485.C >>>> >>>> diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc >>>> index e925ba9fad9..e102305c90c 100644 >>>> --- a/gcc/config/rs6000/rs6000-builtin.cc >>>> +++ b/gcc/config/rs6000/rs6000-builtin.cc >>>> @@ -1294,6 +1294,11 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi) >>>> enum tree_code bcode; >>>> gimple *g; >>>> >>>> + /* For an unresolved overloaded builtin, return early here since there >>>> + is no builtin info for it and we are unable to fold it. */ >>>> + if (fn_code > RS6000_OVLD_NONE) >>>> + return false; >>>> + >>>> size_t uns_fncode = (size_t) fn_code; >>>> enum insn_code icode = rs6000_builtin_info[uns_fncode].icode; >>>> const char *fn_name1 = rs6000_builtin_info[uns_fncode].bifname; >>>> @@ -3295,6 +3300,14 @@ rs6000_expand_builtin (tree exp, rtx target, rtx /* subtarget */, >>>> tree fndecl = TREE_OPERAND (CALL_EXPR_FN (exp), 0); >>>> enum rs6000_gen_builtins fcode >>>> = (enum rs6000_gen_builtins) DECL_MD_FUNCTION_CODE (fndecl); >>>> + >>>> + /* Emit error message if it's an unresolved overloaded builtin. */ >>>> + if (fcode > RS6000_OVLD_NONE) >>>> + { >>>> + error ("unresolved overload for builtin %qF", fndecl); >>>> + return const0_rtx; >>>> + } >>>> + >>>> size_t uns_fcode = (size_t)fcode; >>>> enum insn_code icode = rs6000_builtin_info[uns_fcode].icode; >>>> >>>> diff --git a/gcc/testsuite/g++.target/powerpc/pr105485.C b/gcc/testsuite/g++.target/powerpc/pr105485.C >>>> new file mode 100644 >>>> index 00000000000..a3b8290df8c >>>> --- /dev/null >>>> +++ b/gcc/testsuite/g++.target/powerpc/pr105485.C >>>> @@ -0,0 +1,9 @@ >>>> +/* It's to verify no ICE here, ignore error/warning messages since >>>> + they are not test points here. */ >>>> +/* { dg-excess-errors "pr105485" } */ >>>> + >>>> +template <class> void __builtin_vec_vslv(); >>>> +typedef __attribute__((altivec(vector__))) char T; >>>> +T b (T c, T d) { >>>> + return __builtin_vec_vslv(c, d); >>>> +}
Hi, Gentle ping https://gcc.gnu.org/pipermail/gcc-patches/2022-May/594699.html I think this is a reasonable fix, the behavior is consistent with what we have in the previous built-in framework, I'm going to push this a week later if no objections. :) BR, Kewen >>> >>>> on 2022/5/13 13:29, Kewen.Lin via Gcc-patches wrote: >>>>> Hi, >>>>> >>>>> PR105485 exposes that new builtin function framework doesn't handle >>>>> unresolved overloaded builtin function well. With new builtin >>>>> function support, we don't have builtin info for any overloaded >>>>> rs6000_gen_builtins enum, since they are expected to be resolved to >>>>> one specific instance. So when function rs6000_gimple_fold_builtin >>>>> faces one unresolved overloaded builtin, the access for builtin info >>>>> becomes out of bound and gets ICE then. >>>>> >>>>> We should not try to fold one unresolved overloaded builtin there >>>>> and as the previous support we should emit one error message during >>>>> expansion phase like "unresolved overload for builtin ...". >>>>> >>>>> Bootstrapped and regtested on powerpc64-linux-gnu P8 and >>>>> powerpc64le-linux-gnu P9 and P10. >>>>> >>>>> Is it ok for trunk? >>>>> >>>>> BR, >>>>> Kewen >>>>> ----- >>>>> PR target/105485 >>>>> >>>>> gcc/ChangeLog: >>>>> >>>>> * config/rs6000/rs6000-builtin.cc (rs6000_gimple_fold_builtin): Add >>>>> the handling for unresolved overloaded builtin function. >>>>> (rs6000_expand_builtin): Likewise. >>>>> >>>>> gcc/testsuite/ChangeLog: >>>>> >>>>> * g++.target/powerpc/pr105485.C: New test. >>>>> >>>>> --- >>>>> gcc/config/rs6000/rs6000-builtin.cc | 13 +++++++++++++ >>>>> gcc/testsuite/g++.target/powerpc/pr105485.C | 9 +++++++++ >>>>> 2 files changed, 22 insertions(+) >>>>> create mode 100644 gcc/testsuite/g++.target/powerpc/pr105485.C >>>>> >>>>> diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc >>>>> index e925ba9fad9..e102305c90c 100644 >>>>> --- a/gcc/config/rs6000/rs6000-builtin.cc >>>>> +++ b/gcc/config/rs6000/rs6000-builtin.cc >>>>> @@ -1294,6 +1294,11 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi) >>>>> enum tree_code bcode; >>>>> gimple *g; >>>>> >>>>> + /* For an unresolved overloaded builtin, return early here since there >>>>> + is no builtin info for it and we are unable to fold it. */ >>>>> + if (fn_code > RS6000_OVLD_NONE) >>>>> + return false; >>>>> + >>>>> size_t uns_fncode = (size_t) fn_code; >>>>> enum insn_code icode = rs6000_builtin_info[uns_fncode].icode; >>>>> const char *fn_name1 = rs6000_builtin_info[uns_fncode].bifname; >>>>> @@ -3295,6 +3300,14 @@ rs6000_expand_builtin (tree exp, rtx target, rtx /* subtarget */, >>>>> tree fndecl = TREE_OPERAND (CALL_EXPR_FN (exp), 0); >>>>> enum rs6000_gen_builtins fcode >>>>> = (enum rs6000_gen_builtins) DECL_MD_FUNCTION_CODE (fndecl); >>>>> + >>>>> + /* Emit error message if it's an unresolved overloaded builtin. */ >>>>> + if (fcode > RS6000_OVLD_NONE) >>>>> + { >>>>> + error ("unresolved overload for builtin %qF", fndecl); >>>>> + return const0_rtx; >>>>> + } >>>>> + >>>>> size_t uns_fcode = (size_t)fcode; >>>>> enum insn_code icode = rs6000_builtin_info[uns_fcode].icode; >>>>> >>>>> diff --git a/gcc/testsuite/g++.target/powerpc/pr105485.C b/gcc/testsuite/g++.target/powerpc/pr105485.C >>>>> new file mode 100644 >>>>> index 00000000000..a3b8290df8c >>>>> --- /dev/null >>>>> +++ b/gcc/testsuite/g++.target/powerpc/pr105485.C >>>>> @@ -0,0 +1,9 @@ >>>>> +/* It's to verify no ICE here, ignore error/warning messages since >>>>> + they are not test points here. */ >>>>> +/* { dg-excess-errors "pr105485" } */ >>>>> + >>>>> +template <class> void __builtin_vec_vslv(); >>>>> +typedef __attribute__((altivec(vector__))) char T; >>>>> +T b (T c, T d) { >>>>> + return __builtin_vec_vslv(c, d); >>>>> +}
diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc index e925ba9fad9..e102305c90c 100644 --- a/gcc/config/rs6000/rs6000-builtin.cc +++ b/gcc/config/rs6000/rs6000-builtin.cc @@ -1294,6 +1294,11 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi) enum tree_code bcode; gimple *g; + /* For an unresolved overloaded builtin, return early here since there + is no builtin info for it and we are unable to fold it. */ + if (fn_code > RS6000_OVLD_NONE) + return false; + size_t uns_fncode = (size_t) fn_code; enum insn_code icode = rs6000_builtin_info[uns_fncode].icode; const char *fn_name1 = rs6000_builtin_info[uns_fncode].bifname; @@ -3295,6 +3300,14 @@ rs6000_expand_builtin (tree exp, rtx target, rtx /* subtarget */, tree fndecl = TREE_OPERAND (CALL_EXPR_FN (exp), 0); enum rs6000_gen_builtins fcode = (enum rs6000_gen_builtins) DECL_MD_FUNCTION_CODE (fndecl); + + /* Emit error message if it's an unresolved overloaded builtin. */ + if (fcode > RS6000_OVLD_NONE) + { + error ("unresolved overload for builtin %qF", fndecl); + return const0_rtx; + } + size_t uns_fcode = (size_t)fcode; enum insn_code icode = rs6000_builtin_info[uns_fcode].icode; diff --git a/gcc/testsuite/g++.target/powerpc/pr105485.C b/gcc/testsuite/g++.target/powerpc/pr105485.C new file mode 100644 index 00000000000..a3b8290df8c --- /dev/null +++ b/gcc/testsuite/g++.target/powerpc/pr105485.C @@ -0,0 +1,9 @@ +/* It's to verify no ICE here, ignore error/warning messages since + they are not test points here. */ +/* { dg-excess-errors "pr105485" } */ + +template <class> void __builtin_vec_vslv(); +typedef __attribute__((altivec(vector__))) char T; +T b (T c, T d) { + return __builtin_vec_vslv(c, d); +}