| Message ID | 20151029130803.GB63456@msticlxl57.ims.intel.com |
|---|---|
| State | New |
| Headers | show |
On 10/29/2015 07:08 AM, Ilya Enkovich wrote: > On 28 Oct 22:37, Ilya Enkovich wrote: >> Seems the problem occurs in this check in expand_vector_operations_1: >> >> /* A scalar operation pretending to be a vector one. */ >> if (VECTOR_BOOLEAN_TYPE_P (type) >> && !VECTOR_MODE_P (TYPE_MODE (type)) >> && TYPE_MODE (type) != BLKmode) >> return; >> >> This is to filter out scalar operations on boolean vectors. >> The problem here is that TYPE_MODE (type) doesn't return >> V4SImode assigned to the type but calls vector_type_mode >> instead which tries to find an integer mode for it and returns >> TImode. This causes function exit and we don't expand vector >> comparison. >> >> Suppose simple option to fix it is to change default get_mask_mode >> hook to return BLKmode in case chosen integer vector mode is not >> vector_mode_supported_p. >> >> Thanks, >> Ilya >> > > Here is a patch which fixes the problem on ARM (and on i386 with -mno-sse also). I checked it fixes the problem on ARM and also bootstrapped and checked it on x86_64-unknown-linux-gnu. Is it OK? > > Thanks, > Ilya > -- > gcc/ > > 2015-10-29 Ilya Enkovich <enkovich.gnu@gmail.com> > > * targhooks.c (default_get_mask_mode): Use BLKmode in > case target doesn't support required vector mode. > * stor-layout.c (layout_type): Check for BLKmode. VOIDmode would probably be a better choice than BLKmode to signal when the target doesn't support the required vector mode. Jeff
On Mon, Nov 2, 2015 at 8:41 PM, Jeff Law <law@redhat.com> wrote: > On 10/29/2015 07:08 AM, Ilya Enkovich wrote: >> >> On 28 Oct 22:37, Ilya Enkovich wrote: >>> >>> Seems the problem occurs in this check in expand_vector_operations_1: >>> >>> /* A scalar operation pretending to be a vector one. */ >>> if (VECTOR_BOOLEAN_TYPE_P (type) >>> && !VECTOR_MODE_P (TYPE_MODE (type)) >>> && TYPE_MODE (type) != BLKmode) >>> return; >>> >>> This is to filter out scalar operations on boolean vectors. >>> The problem here is that TYPE_MODE (type) doesn't return >>> V4SImode assigned to the type but calls vector_type_mode >>> instead which tries to find an integer mode for it and returns >>> TImode. This causes function exit and we don't expand vector >>> comparison. >>> >>> Suppose simple option to fix it is to change default get_mask_mode >>> hook to return BLKmode in case chosen integer vector mode is not >>> vector_mode_supported_p. >>> >>> Thanks, >>> Ilya >>> >> >> Here is a patch which fixes the problem on ARM (and on i386 with -mno-sse >> also). I checked it fixes the problem on ARM and also bootstrapped and >> checked it on x86_64-unknown-linux-gnu. Is it OK? >> >> Thanks, >> Ilya >> -- >> gcc/ >> >> 2015-10-29 Ilya Enkovich <enkovich.gnu@gmail.com> >> >> * targhooks.c (default_get_mask_mode): Use BLKmode in >> case target doesn't support required vector mode. >> * stor-layout.c (layout_type): Check for BLKmode. > > VOIDmode would probably be a better choice than BLKmode to signal when the > target doesn't support the required vector mode. Though we're using BLKmode vectors in all other cases to signal that. Richard. > > Jeff >
On 11/03/2015 04:26 AM, Richard Biener wrote: > On Mon, Nov 2, 2015 at 8:41 PM, Jeff Law <law@redhat.com> wrote: >> On 10/29/2015 07:08 AM, Ilya Enkovich wrote: >>> >>> On 28 Oct 22:37, Ilya Enkovich wrote: >>>> >>>> Seems the problem occurs in this check in expand_vector_operations_1: >>>> >>>> /* A scalar operation pretending to be a vector one. */ >>>> if (VECTOR_BOOLEAN_TYPE_P (type) >>>> && !VECTOR_MODE_P (TYPE_MODE (type)) >>>> && TYPE_MODE (type) != BLKmode) >>>> return; >>>> >>>> This is to filter out scalar operations on boolean vectors. >>>> The problem here is that TYPE_MODE (type) doesn't return >>>> V4SImode assigned to the type but calls vector_type_mode >>>> instead which tries to find an integer mode for it and returns >>>> TImode. This causes function exit and we don't expand vector >>>> comparison. >>>> >>>> Suppose simple option to fix it is to change default get_mask_mode >>>> hook to return BLKmode in case chosen integer vector mode is not >>>> vector_mode_supported_p. >>>> >>>> Thanks, >>>> Ilya >>>> >>> >>> Here is a patch which fixes the problem on ARM (and on i386 with -mno-sse >>> also). I checked it fixes the problem on ARM and also bootstrapped and >>> checked it on x86_64-unknown-linux-gnu. Is it OK? >>> >>> Thanks, >>> Ilya >>> -- >>> gcc/ >>> >>> 2015-10-29 Ilya Enkovich <enkovich.gnu@gmail.com> >>> >>> * targhooks.c (default_get_mask_mode): Use BLKmode in >>> case target doesn't support required vector mode. >>> * stor-layout.c (layout_type): Check for BLKmode. >> >> VOIDmode would probably be a better choice than BLKmode to signal when the >> target doesn't support the required vector mode. > > Though we're using BLKmode vectors in all other cases to signal that. If that's the case in the vetorizer then let's stay consistent with that existing practice in the vectorizer. jeff
On 10/29/2015 07:08 AM, Ilya Enkovich wrote: > On 28 Oct 22:37, Ilya Enkovich wrote: >> Seems the problem occurs in this check in expand_vector_operations_1: >> >> /* A scalar operation pretending to be a vector one. */ >> if (VECTOR_BOOLEAN_TYPE_P (type) >> && !VECTOR_MODE_P (TYPE_MODE (type)) >> && TYPE_MODE (type) != BLKmode) >> return; >> >> This is to filter out scalar operations on boolean vectors. >> The problem here is that TYPE_MODE (type) doesn't return >> V4SImode assigned to the type but calls vector_type_mode >> instead which tries to find an integer mode for it and returns >> TImode. This causes function exit and we don't expand vector >> comparison. >> >> Suppose simple option to fix it is to change default get_mask_mode >> hook to return BLKmode in case chosen integer vector mode is not >> vector_mode_supported_p. >> >> Thanks, >> Ilya >> > > Here is a patch which fixes the problem on ARM (and on i386 with -mno-sse also). I checked it fixes the problem on ARM and also bootstrapped and checked it on x86_64-unknown-linux-gnu. Is it OK? > > Thanks, > Ilya > -- > gcc/ > > 2015-10-29 Ilya Enkovich <enkovich.gnu@gmail.com> > > * targhooks.c (default_get_mask_mode): Use BLKmode in > case target doesn't support required vector mode. > * stor-layout.c (layout_type): Check for BLKmode. And just to be clear, since Richi pointed out that we're already using BLKmode for this kind of situation, this patch is OK. Jeff
diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c index 58ecd7b..ae7d6fb 100644 --- a/gcc/stor-layout.c +++ b/gcc/stor-layout.c @@ -2185,7 +2185,8 @@ layout_type (tree type) TYPE_SATURATING (type) = TYPE_SATURATING (TREE_TYPE (type)); TYPE_UNSIGNED (type) = TYPE_UNSIGNED (TREE_TYPE (type)); /* Several boolean vector elements may fit in a single unit. */ - if (VECTOR_BOOLEAN_TYPE_P (type)) + if (VECTOR_BOOLEAN_TYPE_P (type) + && type->type_common.mode != BLKmode) TYPE_SIZE_UNIT (type) = size_int (GET_MODE_SIZE (type->type_common.mode)); else diff --git a/gcc/targhooks.c b/gcc/targhooks.c index c39f266..d378864 100644 --- a/gcc/targhooks.c +++ b/gcc/targhooks.c @@ -1095,10 +1095,16 @@ default_get_mask_mode (unsigned nunits, unsigned vector_size) unsigned elem_size = vector_size / nunits; machine_mode elem_mode = smallest_mode_for_size (elem_size * BITS_PER_UNIT, MODE_INT); + machine_mode vector_mode; gcc_assert (elem_size * nunits == vector_size); - return mode_for_vector (elem_mode, nunits); + vector_mode = mode_for_vector (elem_mode, nunits); + if (VECTOR_MODE_P (vector_mode) + && !targetm.vector_mode_supported_p (vector_mode)) + vector_mode = BLKmode; + + return vector_mode; } /* By default, the cost model accumulates three separate costs (prologue,