| Message ID | 1518800329.7508.37.camel@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [rs6000] Require Power 8 for vec_float2 builtin | expand |
Hi! On Fri, Feb 16, 2018 at 08:58:49AM -0800, Carl Love wrote: > This patch changes the expansion macros to the equivalent Power 8 > macros. The test now cleanly exits with the message > > error: builtin function ‘__builtin_vsx_float2_v2di’ requires the ‘- > mpower8-vector’ option > > rather then giving an internal compiler error when compiled with the > cpu=power7 option. :-) > The patch was tested by running the full regression suite to ensure no > new regressions were introduced. Additionally, the patch was tested by > hand compiling with the -mcpu=power7 option to verify the issue is > fixed. The testing was don on powerpc64le-unknown-linux-gnu (Power 8 > LE) Do you have a new testcase, too? Or is this all covered by existing tests (which?) Looks great, thanks! Okay for trunk (and for backports after a while, if needed). Segher > * config/rs6000/rs6000-builtin.def: Add BU_P8V_VSX_2 macro definition. > Change FLOAT2 expansions from BU_VSX_2 to BU_P8V_VSX_2 and > from BU_VSX_OVERLOAD_2 to BU_P8V_OVERLOAD_2. > * config/rs6000/rs6000-c.c: Changed macro VSX_BUILTIN_VEC_FLOAT2 > expansion to P8V_BUILTIN_VEC_FLOAT2.
On Fri, 2018-02-16 at 11:56 -0600, Segher Boessenkool wrote: > Hi! Segher: Per our discussion on IRC. I created the bugzilla: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84422 for this issue so it is visible to people outside IBM. I have included this test file int the above bugzilla along with the other four builtin test case files that ICE when compiled with the -mcpu=power7 option. > On Fri, Feb 16, 2018 at 08:58:49AM -0800, Carl Love wrote: > > This patch changes the expansion macros to the equivalent Power 8 > > macros. The test now cleanly exits with the message > > > > error: builtin function ‘__builtin_vsx_float2_v2di’ requires > > the ‘- > > mpower8-vector’ option > > > > rather then giving an internal compiler error when compiled with > > the > > cpu=power7 option. > > :-) > > > The patch was tested by running the full regression suite to ensure > > no > > new regressions were introduced. Additionally, the patch was > > tested by > > hand compiling with the -mcpu=power7 option to verify the issue is > > fixed. The testing was don on powerpc64le-unknown-linux-gnu > > (Power 8 > > LE) > > Do you have a new testcase, too? Or is this all covered by existing > tests (which?) The issue was found when compiling the existing test case file with -mcpu=power7. No additional tests are needed for this issue. > > Looks great, thanks! Okay for trunk (and for backports after a > while, > if needed). Carl Love
On Fri, 2018-02-16 at 10:21 -0800, Carl Love wrote: > > > > > Do you have a new testcase, too? Or is this all covered by > > existing > > tests (which?) > > The issue was found when compiling the existing test case file with > -mcpu=power7. No additional tests are needed for this issue. > Sorry, my bad. The exiting test case file is: gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c > > > > Looks great, thanks! Okay for trunk (and for backports after a > > while, > > if needed). > > Carl Love >
diff --git a/gcc/config/rs6000/rs6000-builtin.def b/gcc/config/rs6000/rs6000-builtin.def index 86604da..05eb356 100644 --- a/gcc/config/rs6000/rs6000-builtin.def +++ b/gcc/config/rs6000/rs6000-builtin.def @@ -403,6 +403,14 @@ | RS6000_BTC_UNARY), \ CODE_FOR_ ## ICODE) /* ICODE */ +#define BU_P8V_VSX_2(ENUM, NAME, ATTR, ICODE) \ + RS6000_BUILTIN_2 (P8V_BUILTIN_ ## ENUM, /* ENUM */ \ + "__builtin_vsx_" NAME, /* NAME */ \ + RS6000_BTM_P8_VECTOR, /* MASK */ \ + (RS6000_BTC_ ## ATTR /* ATTR */ \ + | RS6000_BTC_BINARY), \ + CODE_FOR_ ## ICODE) /* ICODE */ + #define BU_P8V_OVERLOAD_1(ENUM, NAME) \ RS6000_BUILTIN_1 (P8V_BUILTIN_VEC_ ## ENUM, /* ENUM */ \ "__builtin_vec_" NAME, /* NAME */ \ @@ -1659,9 +1667,6 @@ BU_VSX_2 (CMPLE_U16QI, "cmple_u16qi", CONST, vector_ngtuv16qi) BU_VSX_2 (CMPLE_U8HI, "cmple_u8hi", CONST, vector_ngtuv8hi) BU_VSX_2 (CMPLE_U4SI, "cmple_u4si", CONST, vector_ngtuv4si) BU_VSX_2 (CMPLE_U2DI, "cmple_u2di", CONST, vector_ngtuv2di) -BU_VSX_2 (FLOAT2_V2DF, "float2_v2df", CONST, float2_v2df) -BU_VSX_2 (FLOAT2_V2DI, "float2_v2di", CONST, float2_v2di) -BU_VSX_2 (UNS_FLOAT2_V2DI, "uns_float2_v2di", CONST, uns_float2_v2di) BU_VSX_2 (VEC_VSIGNED2_V2DF, "vsigned2_v2df", CONST, vsigned2_v2df) BU_VSX_2 (VEC_VUNSIGNED2_V2DF, "vunsigned2_v2df", CONST, vunsigned2_v2df) @@ -1856,8 +1861,6 @@ BU_VSX_OVERLOAD_2 (XXMRGHW, "xxmrghw") BU_VSX_OVERLOAD_2 (XXMRGLW, "xxmrglw") BU_VSX_OVERLOAD_2 (XXSPLTD, "xxspltd") BU_VSX_OVERLOAD_2 (XXSPLTW, "xxspltw") -BU_VSX_OVERLOAD_2 (FLOAT2, "float2") -BU_VSX_OVERLOAD_2 (UNS_FLOAT2, "uns_float2") BU_VSX_OVERLOAD_2 (VSIGNED2, "vsigned2") BU_VSX_OVERLOAD_2 (VUNSIGNED2, "vunsigned2") @@ -1910,6 +1913,11 @@ BU_P8V_VSX_1 (REVB_V16QI, "revb_v16qi", CONST, revb_v16qi) BU_P8V_VSX_1 (REVB_V2DF, "revb_v2df", CONST, revb_v2df) BU_P8V_VSX_1 (REVB_V4SF, "revb_v4sf", CONST, revb_v4sf) +/* 2 argument VSX instructions added in ISA 2.07. */ +BU_P8V_VSX_2 (FLOAT2_V2DF, "float2_v2df", CONST, float2_v2df) +BU_P8V_VSX_2 (FLOAT2_V2DI, "float2_v2di", CONST, float2_v2di) +BU_P8V_VSX_2 (UNS_FLOAT2_V2DI, "uns_float2_v2di", CONST, uns_float2_v2di) + /* 1 argument altivec instructions added in ISA 2.07. */ BU_P8V_AV_1 (ABS_V2DI, "abs_v2di", CONST, absv2di2) BU_P8V_AV_1 (VUPKHSW, "vupkhsw", CONST, altivec_vupkhsw) @@ -2052,6 +2060,8 @@ BU_P8V_OVERLOAD_2 (VSRD, "vsrd") BU_P8V_OVERLOAD_2 (VSUBCUQ, "vsubcuq") BU_P8V_OVERLOAD_2 (VSUBUDM, "vsubudm") BU_P8V_OVERLOAD_2 (VSUBUQM, "vsubuqm") +BU_P8V_OVERLOAD_2 (FLOAT2, "float2") +BU_P8V_OVERLOAD_2 (UNS_FLOAT2, "uns_float2") /* ISA 2.07 vector overloaded 3 argument functions. */ BU_P8V_OVERLOAD_3 (VADDECUQ, "vaddecuq") diff --git a/gcc/config/rs6000/rs6000-c.c b/gcc/config/rs6000/rs6000-c.c index a68be51..236003f 100644 --- a/gcc/config/rs6000/rs6000-c.c +++ b/gcc/config/rs6000/rs6000-c.c @@ -1542,11 +1542,11 @@ const struct altivec_builtin_types altivec_overloaded_builtins[] = { RS6000_BTI_V4SF, RS6000_BTI_V4SI, 0, 0 }, { VSX_BUILTIN_VEC_FLOAT, VSX_BUILTIN_XVCVUXWSP_V4SF, RS6000_BTI_V4SF, RS6000_BTI_unsigned_V4SI, 0, 0 }, - { VSX_BUILTIN_VEC_FLOAT2, VSX_BUILTIN_FLOAT2_V2DF, + { P8V_BUILTIN_VEC_FLOAT2, P8V_BUILTIN_FLOAT2_V2DF, RS6000_BTI_V4SF, RS6000_BTI_V2DF, RS6000_BTI_V2DF, 0 }, - { VSX_BUILTIN_VEC_FLOAT2, VSX_BUILTIN_FLOAT2_V2DI, + { P8V_BUILTIN_VEC_FLOAT2, P8V_BUILTIN_FLOAT2_V2DI, RS6000_BTI_V4SF, RS6000_BTI_V2DI, RS6000_BTI_V2DI, 0 }, - { VSX_BUILTIN_VEC_FLOAT2, VSX_BUILTIN_UNS_FLOAT2_V2DI, + { P8V_BUILTIN_VEC_FLOAT2, P8V_BUILTIN_UNS_FLOAT2_V2DI, RS6000_BTI_V4SF, RS6000_BTI_unsigned_V2DI, RS6000_BTI_unsigned_V2DI, 0 }, { VSX_BUILTIN_VEC_FLOATE, VSX_BUILTIN_FLOATE_V2DF,
GCC maintainers: The following patch fixes a GCC internal compiler error. Several of the builtins use the wrong enable conditions as mentioned in Bill's issue data base (https://github.ibm.com/wschmidt/power-gcc/issue s/282). The implementation for the vec_float2() builtin, as implemented, uses the pre Power 8 macro expansions BU_VSX_2 and BU_VSX_OVERLOAD_2. The result is GCC generates an internal compiler error when the test file is compiled with -mcpu=power7. The builtin support uses RTL code that is only supported on Power 8 and beyond. Additionally, the 64-bit ABI document that lists the vec_float2() builtin says the listed builtins are supported on Power 8 or newer platforms. This patch changes the expansion macros to the equivalent Power 8 macros. The test now cleanly exits with the message error: builtin function ‘__builtin_vsx_float2_v2di’ requires the ‘- mpower8-vector’ option rather then giving an internal compiler error when compiled with the cpu=power7 option. The patch was tested by running the full regression suite to ensure no new regressions were introduced. Additionally, the patch was tested by hand compiling with the -mcpu=power7 option to verify the issue is fixed. The testing was don on powerpc64le-unknown-linux-gnu (Power 8 LE) Please let me know if the patch looks OK or not. Thanks. Carl Love --------------------------------------------------------------------------------- gcc/ChangeLog: 2018-02-16 Carl Love <cel@us.ibm.com> * config/rs6000/rs6000-builtin.def: Add BU_P8V_VSX_2 macro definition. Change FLOAT2 expansions from BU_VSX_2 to BU_P8V_VSX_2 and from BU_VSX_OVERLOAD_2 to BU_P8V_OVERLOAD_2. * config/rs6000/rs6000-c.c: Changed macro VSX_BUILTIN_VEC_FLOAT2 expansion to P8V_BUILTIN_VEC_FLOAT2. --- gcc/config/rs6000/rs6000-builtin.def | 20 +++++++++++++++----- gcc/config/rs6000/rs6000-c.c | 6 +++--- 2 files changed, 18 insertions(+), 8 deletions(-)