On Tue, 2017-01-24 at 11:08 -0600, Segher Boessenkool wrote:
> On Tue, Jan 24, 2017 at 08:28:37AM -0800, Carl E. Love wrote:
> > The following patch fixes an issue with the entries in the table of
> > built-in functions. All of the entries for a given built-in, must occur
> > in the table as a single block of entries. Otherwise the code that
> > searches the table for a given built-in definition will stop looking
> > once it reaches the end of the initial block of definitions for that
> > built-in function and subsequent definitions for that built-in will
> > never be checked. This issue currently occurs with the
> > ALTIVEC_BUILTIN_VEC_PACKS and P8V_BUILTIN_VEC_VGBBD built-ins. The
> > patch simply moves the existing entries so the definition for a given
> > built-in are all together in the same block of entries.
>
> Do we need a separate testcase to check for this? Or do those specific
> builtins need better testcases? Or was the bug obvious already?
I have a list of built-ins that need to have support and test cases
added. I found the issue with the ALTIVEC_BUILTIN_VEC_PACKS when I
tried to add support for the built-ins:
vector signed int vec_packs (vector signed long long x, vector signed long
long y);
vector unsigned int vec_packs (vector unsigned long long x, vector unsigned
long long y);
which were in my to do list. What I found was the support for vec_packs
is all there but I don't find any test cases for these built-ins. At
this point, I do plan to add the vec_pack test cases as part of my work
to add the support for the other built-ins on my list. I have the patch
in my patch series with the others that need adding. Currently holding
off on posting patches since we are only supposed to be posting bug
fixes at the moment.
Once the bug for the ALTIVEC_BUILTIN_VEC_PACKS built-in was found, I
wrote a perl script to scan through the entire table looking for the
issue with any other built-in functions. The script found the issue
with the P8V_BUILTIN_VEC_VGBBD built-in. My list of built-ins to add
doesn't include anything for vec_vgbbd.
It would be easy for my to add the test cases for the vec_packs()
built-ins to this patch if you would like?
I just took a look at the vec_vgbbd() built-in. I grep'd for vgbbd and
found the followint two testcases in
gcc/testsuite/gcc.target/powerpc/p8vector-builtin-4.c:
typedef vector signed char vc_sign;
typedef vector unsigned char vc_uns;
vc_sign vc_gbb_2 (vc_sign a)
{
return vec_vgbbd (a);
}
vc_uns vc_gbb_3 (vc_uns a)
{
return vec_vgbbd (a);
}
which correspond to the built-in entries in rs6000-c.c which I didn't move
{ P8V_BUILTIN_VEC_VGBBD, P8V_BUILTIN_VGBBD,
RS6000_BTI_V16QI, RS6000_BTI_V16QI, 0, 0 },
{ P8V_BUILTIN_VEC_VGBBD, P8V_BUILTIN_VGBBD,
RS6000_BTI_unsigned_V16QI, RS6000_BTI_unsigned_V16QI, 0, 0 },
I don't see any tests for the two built-in entries in rs6000-c.c which the
patch moves, i.e.
{ P8V_BUILTIN_VEC_VGBBD, P8V_BUILTIN_VGBBD,
RS6000_BTI_V16QI, 0, 0, 0 },
{ P8V_BUILTIN_VEC_VGBBD, P8V_BUILTIN_VGBBD,
RS6000_BTI_unsigned_V16QI, 0, 0, 0 },
I tried a quick test of adding the following to the test file
p8vector-builtin-4.c for these entries:
vc_sign vc_gbb_4 (void)
{
return vec_vgbbd ();
}
vc_uns vc_gbb_5 (void)
{
return vec_vgbbd ();
}
When I compile I get " error: too few arguments to function
‘__builtin_vec_vgbbd’ ".
At the moment, I am not sure the point is of a function that returns
something but doesn't take any arguments??? I don't see vgbbd listed in the
ABI document
so not sure if the two built-ins that don't take any arguments are really valid
builtins?
I would be willing add the above two test cases to the patch if we can figure
out how to
get them to work.
Please let me know if you want me to go ahead with the adding the vec_packs()
test cases
to the patch or not. Again, not so sure about the vec_vgbbd test cases.
Carl Love
