[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532 kelvin at gcc dot gnu.org changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #20 from kelvin at gcc dot gnu.org --- Patch has been committed to trunk and backported to gcc8 and gcc7.
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532 --- Comment #19 from kelvin at gcc dot gnu.org --- Author: kelvin Date: Mon Apr 22 16:09:13 2019 New Revision: 270493 URL: https://gcc.gnu.org/viewcvs?rev=270493&root=gcc&view=rev Log: gcc/ChangeLog: 2019-04-22 Kelvin Nilsen Backport from mainline 2019-03-15 Kelvin Nilsen PR target/87532 * config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin): When handling vec_extract, use modular arithmetic to allow constant selectors greater than vector length. * config/rs6000/rs6000.c (rs6000_expand_vector_extract): Allow V1TImode vectors to have constant selector values greater than 0. Use modular arithmetic to compute vector index. (rs6000_split_vec_extract_var): Use modular arithmetic to compute index for in-memory vectors. Correct code generation for in-register vectors. Use inner mode of vector rather than mode of destination for move instruction. (altivec_expand_vec_ext_builtin): Use modular arithmetic to compute index. 2019-04-12 Kelvin Nilsen PR target/87532 * config/rs6000/vsx.md (*vsx_extract__mode_var): Use QI inner mode with V16QI vector mode. gcc/testsuite/ChangeLog: 2019-04-22 Kelvin Nilsen Backport from mainline 2019-03-15 Kelvin Nilsen PR target/87532 * gcc.target/powerpc/pr87532-mc.c: New test. * gcc.target/powerpc/pr87532.c: New test. * gcc.target/powerpc/vec-extract-v16qiu-v2.h: New test. * gcc.target/powerpc/vec-extract-v16qiu-v2a.c: New test. * gcc.target/powerpc/vec-extract-v16qiu-v2b.c: New test. * gcc.target/powerpc/vsx-builtin-10a.c: New test. * gcc.target/powerpc/vsx-builtin-10b.c: New test. * gcc.target/powerpc/vsx-builtin-11a.c: New test. * gcc.target/powerpc/vsx-builtin-11b.c: New test. * gcc.target/powerpc/vsx-builtin-12a.c: New test. * gcc.target/powerpc/vsx-builtin-12b.c: New test. * gcc.target/powerpc/vsx-builtin-13a.c: New test. * gcc.target/powerpc/vsx-builtin-13b.c: New test. * gcc.target/powerpc/vsx-builtin-14a.c: New test. * gcc.target/powerpc/vsx-builtin-14b.c: New test. * gcc.target/powerpc/vsx-builtin-15a.c: New test. * gcc.target/powerpc/vsx-builtin-15b.c: New test. * gcc.target/powerpc/vsx-builtin-16a.c: New test. * gcc.target/powerpc/vsx-builtin-16b.c: New test. * gcc.target/powerpc/vsx-builtin-17a.c: New test. * gcc.target/powerpc/vsx-builtin-17b.c: New test. * gcc.target/powerpc/vsx-builtin-18a.c: New test. * gcc.target/powerpc/vsx-builtin-18b.c: New test. * gcc.target/powerpc/vsx-builtin-19a.c: New test. * gcc.target/powerpc/vsx-builtin-19b.c: New test. * gcc.target/powerpc/vsx-builtin-20a.c: New test. * gcc.target/powerpc/vsx-builtin-20b.c: New test. * gcc.target/powerpc/vsx-builtin-9a.c: New test. * gcc.target/powerpc/vsx-builtin-9b.c: New test. 2019-03-19 Kelvin Nilsen PR target/89736 * gcc.target/powerpc/pr87532-mc.c: Modify dejagnu directives to restrict this test to vsx targets. Added: branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/pr87532-mc.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/pr87532.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2.h branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2a.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2b.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-10a.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-10b.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-11a.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-11b.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-12a.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-12b.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-13a.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-13b.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-14a.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-14b.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-15a.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-15b.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-16a.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-16b.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-17a.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-17b.c branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/vsx-bui
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532 --- Comment #18 from kelvin at gcc dot gnu.org --- Author: kelvin Date: Wed Apr 17 15:40:12 2019 New Revision: 270413 URL: https://gcc.gnu.org/viewcvs?rev=270413&root=gcc&view=rev Log: gcc/ChangeLog: 2019-04-17 Kelvin Nilsen Backport from mainline 2019-03-15 Kelvin Nilsen PR target/87532 * config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin): When handling vec_extract, use modular arithmetic to allow constant selectors greater than vector length. * config/rs6000/rs6000.c (rs6000_expand_vector_extract): Allow V1TImode vectors to have constant selector values greater than 0. Use modular arithmetic to compute vector index. (rs6000_split_vec_extract_var): Use modular arithmetic to compute index for in-memory vectors. Correct code generation for in-register vectors. Use inner mode of vector rather than mode of destination for move instruction. (altivec_expand_vec_ext_builtin): Use modular arithmetic to compute index. 2019-04-12 Kelvin Nilsen PR target/87532 * config/rs6000/vsx.md (*vsx_extract__mode_var): Use QI inner mode with V16QI vector mode. gcc/testsuite/ChangeLog: 2019-04-17 Kelvin Nilsen Backport from mainline 2019-03-15 Kelvin Nilsen PR target/87532 * gcc.target/powerpc/pr87532.c: New test. * gcc.target/powerpc/vec-extract-v16qiu-v2.h: New test. * gcc.target/powerpc/vec-extract-v16qiu-v2a.c: New test. * gcc.target/powerpc/vec-extract-v16qiu-v2b.c: New test. * gcc.target/powerpc/vsx-builtin-10a.c: New test. * gcc.target/powerpc/vsx-builtin-10b.c: New test. * gcc.target/powerpc/vsx-builtin-11a.c: New test. * gcc.target/powerpc/vsx-builtin-11b.c: New test. * gcc.target/powerpc/vsx-builtin-12a.c: New test. * gcc.target/powerpc/vsx-builtin-12b.c: New test. * gcc.target/powerpc/vsx-builtin-13a.c: New test. * gcc.target/powerpc/vsx-builtin-13b.c: New test. * gcc.target/powerpc/vsx-builtin-14a.c: New test. * gcc.target/powerpc/vsx-builtin-14b.c: New test. * gcc.target/powerpc/vsx-builtin-15a.c: New test. * gcc.target/powerpc/vsx-builtin-15b.c: New test. * gcc.target/powerpc/vsx-builtin-16a.c: New test. * gcc.target/powerpc/vsx-builtin-16b.c: New test. * gcc.target/powerpc/vsx-builtin-17a.c: New test. * gcc.target/powerpc/vsx-builtin-17b.c: New test. * gcc.target/powerpc/vsx-builtin-18a.c: New test. * gcc.target/powerpc/vsx-builtin-18b.c: New test. * gcc.target/powerpc/vsx-builtin-19a.c: New test. * gcc.target/powerpc/vsx-builtin-19b.c: New test. * gcc.target/powerpc/vsx-builtin-20a.c: New test. * gcc.target/powerpc/vsx-builtin-20b.c: New test. * gcc.target/powerpc/vsx-builtin-9a.c: New test. * gcc.target/powerpc/vsx-builtin-9b.c: New test. 2019-03-19 Kelvin Nilsen PR target/89736 * gcc.target/powerpc/pr87532-mc.c: Modify dejagnu directives to restrict this test to vsx targets. Added: branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/pr87532-mc.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/pr87532.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2.h branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2a.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2b.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-10a.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-10b.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-11a.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-11b.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-12a.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-12b.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-13a.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-13b.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-14a.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-14b.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-15a.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-15b.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-16a.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-16b.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-17a.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-17b.c branches/gcc-8-branch/gcc/testsuite/gcc.target/powerpc/vsx-builtin-18a.c branches/gcc-8-branch/gcc/testsuite/gc
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532 --- Comment #17 from kelvin at gcc dot gnu.org --- Author: kelvin Date: Fri Apr 12 12:51:58 2019 New Revision: 270313 URL: https://gcc.gnu.org/viewcvs?rev=270313&root=gcc&view=rev Log: gcc/ChangeLog: 2019-04-12 Kelvin Nilsen PR target/87532 * config/rs6000/rs6000.c (rs6000_split_vec_extract_var): Use inner mode of vector rather than mode of destination for move instruction. * config/rs6000/vsx.md (*vsx_extract__mode_var): Use QI inner mode with V16QI vector mode. gcc/testsuite/ChangeLog: 2019-04-12 Kelvin Nilsen PR target/87532 * gcc.target/powerpc/fold-vec-extract-char.p8.c: Adjust expected instruction counts. * gcc.target/powerpc/fold-vec-extract-int.p8.c: Likewise. * gcc.target/powerpc/fold-vec-extract-short.p8.c: Likewise. Modified: trunk/gcc/ChangeLog trunk/gcc/config/rs6000/rs6000.c trunk/gcc/config/rs6000/vsx.md trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-char.p8.c trunk/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-int.p8.c trunk/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-short.p8.c
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532 --- Comment #16 from kelvin at gcc dot gnu.org --- Author: kelvin Date: Fri Mar 15 19:52:43 2019 New Revision: 269715 URL: https://gcc.gnu.org/viewcvs?rev=269715&root=gcc&view=rev Log: gcc/ChangeLog: 2019-03-15 Kelvin Nilsen PR target/87532 * config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin): When handling vec_extract, use modular arithmetic to allow constant selectors greater than vector length. * config/rs6000/rs6000.c (rs6000_expand_vector_extract): Allow V1TImode vectors to have constant selector values greater than 0. Use modular arithmetic to compute vector index. (rs6000_split_vec_extract_var): Use modular arithmetic to compute index for in-memory vectors. Correct code generation for in-register vectors. (altivec_expand_vec_ext_builtin): Use modular arithmetic to compute index. gcc/testsuite/ChangeLog: 2019-03-15 Kelvin Nilsen PR target/87532 * gcc.target/powerpc/fold-vec-extract-char.p8.c: Modify expected instruction selection. * gcc.target/powerpc/fold-vec-extract-int.p8.c: Likewise. * gcc.target/powerpc/fold-vec-extract-short.p8.c: Likewise. * gcc.target/powerpc/pr87532-mc.c: New test. * gcc.target/powerpc/pr87532.c: New test. * gcc.target/powerpc/vec-extract-v16qiu-v2.h: New test. * gcc.target/powerpc/vec-extract-v16qiu-v2a.c: New test. * gcc.target/powerpc/vec-extract-v16qiu-v2b.c: New test. * gcc.target/powerpc/vsx-builtin-10a.c: New test. * gcc.target/powerpc/vsx-builtin-10b.c: New test. * gcc.target/powerpc/vsx-builtin-11a.c: New test. * gcc.target/powerpc/vsx-builtin-11b.c: New test. * gcc.target/powerpc/vsx-builtin-12a.c: New test. * gcc.target/powerpc/vsx-builtin-12b.c: New test. * gcc.target/powerpc/vsx-builtin-13a.c: New test. * gcc.target/powerpc/vsx-builtin-13b.c: New test. * gcc.target/powerpc/vsx-builtin-14a.c: New test. * gcc.target/powerpc/vsx-builtin-14b.c: New test. * gcc.target/powerpc/vsx-builtin-15a.c: New test. * gcc.target/powerpc/vsx-builtin-15b.c: New test. * gcc.target/powerpc/vsx-builtin-16a.c: New test. * gcc.target/powerpc/vsx-builtin-16b.c: New test. * gcc.target/powerpc/vsx-builtin-17a.c: New test. * gcc.target/powerpc/vsx-builtin-17b.c: New test. * gcc.target/powerpc/vsx-builtin-18a.c: New test. * gcc.target/powerpc/vsx-builtin-18b.c: New test. * gcc.target/powerpc/vsx-builtin-19a.c: New test. * gcc.target/powerpc/vsx-builtin-19b.c: New test. * gcc.target/powerpc/vsx-builtin-20a.c: New test. * gcc.target/powerpc/vsx-builtin-20b.c: New test. * gcc.target/powerpc/vsx-builtin-9a.c: New test. * gcc.target/powerpc/vsx-builtin-9b.c: New test. Added: trunk/gcc/testsuite/gcc.target/powerpc/pr87532-mc.c trunk/gcc/testsuite/gcc.target/powerpc/pr87532.c trunk/gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2.h trunk/gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2a.c trunk/gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2b.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-10a.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-10b.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-11a.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-11b.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-12a.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-12b.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-13a.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-13b.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-14a.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-14b.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-15a.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-15b.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-16a.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-16b.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-17a.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-17b.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-18a.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-18b.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-19a.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-19b.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-20a.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-20b.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-9a.c trunk/gcc/testsuite/gcc.target/powerpc/vsx-builtin-9b.c Modified: trunk/gcc/ChangeLog trunk/gcc/config/rs6000/rs6000-c.c trunk/gcc/config/rs6000/rs6000.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-char.p8.c trunk/gcc/testsuite/gcc.target
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532 --- Comment #15 from Bill Schmidt --- I kindasorta thought that's what I want. ;-) But now that I understand what you're saying, I believe I agree with you that this is probably a problem in our gimple folding. I am going to shut up now and stay out of your way, as I believe you have a much better grasp on this than I do at the moment...
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532
--- Comment #14 from kelvin at gcc dot gnu.org ---
To reconcile comments 12 and 13, the subtle issue is that we don't even get
into the altivec_resolve_overloaded_builtin function for the following code:
vec_extract (vi, 10);
The gimple expander replaces this code with BIT_FIELD_REF <> before even
attempting to expand this overloaded builtin.
However, in the case that the "identical" code is represented by inlined
function X, we do come through altivec_resolve_overloaded_builtin.
int X (vector int vi, int index) {
return vec_extract (vi, index);
}
...
X (vi, 10)
There are "two" ways to fix this problem. Either we can make the gimple
expansion of in-lined functions match the non-inlined functions, or we can
insert code into altivec_resolve_overloaded_builtin to accommodate the
inconsistencies.
I gather you prefer I address this within altivec_resolve_overloaded_builtin.
I'll pursue this unless directed otherwise.
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532 --- Comment #13 from Bill Schmidt --- Yes, please look at my previous comment. I believe the problem is in the VEC_EXTRACT processing in rs6000-c.c until proven otherwise... can you please try my debugging suggestion?
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532 --- Comment #12 from kelvin at gcc dot gnu.org --- After further digging, I have uncovered some additional clues: after initial gimple expansion (i.e. the 005t.gimple trace file): vec_extract (vi, 3) is represented by __builtin_vec_ext_v4si (vi, 3) But vec_extract (vi, 10) is represented by __BIT_FIELD_REF Apparently, the difference in handling of these two cases is that the selector argument of the latter is known to be a constant greater than the number of elements in the vector. However, when the same code is expanded after in-lining the doextractbybuiltin function of comment #4, the two expressions expand into __builtin_vec_ext_v4si (vi, 3) and __builtin_vec_ext_v4si (vi, 10) respectively. This behavior is first manifest in the .047t.local-fnsummary2 trace file. Later, when we "process" __builtin_vec_ext_v4si with a constant selector whose value exceeds the vector length, we issue the erroneous error message. With non-constant selector values for __builtin_vec_ext_v4si, we do not issue the error message. I think the place to fix this is in the processing of the __builtin_vec_ext_v4si function (and all of its lookalikes). Rather than issue the error message, we may have to emit slightly different implementations of the code or maybe not even that. Maybe i can just remove the error message. I need to study this a little more. At the same time, I'm wondering if the "real problem" is in the local-fnsummary2 pass. During in-lining, it could be argued that it should have produced the same intermediate form as the original gimple expansion: BIT_FIELD_REF instead of __builtin_vec_ext_v4si. Does anyone have further suggestions before I begin implementing a "solution"? Thanks.
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532
--- Comment #11 from Bill Schmidt ---
Let me take back what I said earlier. We've had full support for vec_extract
with a variable second argument for quite a long time. So let me try again
responding to comment #4.
We have special-case code for ALTIVEC_BUILTIN_VEC_EXTRACT in rs6000-c.c when
handling overloaded built-ins. There is short-circuit code there for cases
where the second argument is a constant and in-range, as well for cases where
the second argument is variable and we have a direct-move instruction
available. In all other cases, we are supposed to expand this code into
address arithmetic:
/* Build *(((arg1_inner_type*)&(vector type){arg1})+arg2). */
But the fact that you are seeing the error message about the selector being out
of range indicates that we are expanding the vector built-in via rs6000.c:
altivec_expand_builtin, which in turn means that rs6000_overloaded_builtin_p
failed to expand the call.
We need to understand why the code in altivec_resolve_overloaded_builtin is not
being expanded as expected. So please set a breakpoint there and look at the
fndecl to see what's going on.
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532 --- Comment #10 from Bill Schmidt --- Hm. Hang on while I look at some history.
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532
--- Comment #9 from kelvin at gcc dot gnu.org ---
The new tests proposed by as part of this PR represent illegal code and are
properly rejected by the compiler.
However, the compiler is not currently rejecting the following test program
even though the test depends on undefined behavior. This test is defined in
vec-extract.h, which is included by 14 expansions of vec-extract-v*.c.
/* Tests for vec_extract where the vector comes from memory (the compiler can
optimize this by doing a scalar load without having to load the whole
vector). */
RTYPE
get_pointer_n (vector TYPE *p, ssize_t n)
{
return (RTYPE) vec_extract (*p, n);
}
...
void
do_pointer (vector TYPE *p)
{
size_t i;
for (i = 0; i < sizeof (get_pointer_const) / sizeof (get_pointer_const[0]);
i\
++)
{
TRACE ("pointer", i);
check (get_pointer_n (p, i), (get_pointer_const[i]) (p));
}
}
This is the code from which the newly proposed tests were derived.
I am inclined to remove this code from the test suite under the principle that
optimizers should not change the legality of code. If the code is illegal
without optimization, it should still be illegal with optimization. In that
spirit, I would think we do not ever want to allow non-constant values as the
selector argument to vec_extract.
I haven't yet confirmed whether the compiler considers this "legal" only
because it fully unrolls the loop and in-lines get_pointer_n or if it is
considered legal because, given a pointer to a vector, the expansion uses a
different implementation technique than it would use in the case of direct move
for an in-register vector.
If we do consider it legal to use vec_extract with a variable selector on
in-memory vectors, then I would want to make sure the semantics is the same
with regards to modular truncation of the selector expression...
I couldn't resist glancing at the implementation of vec_extract in rs6000-c.c.
I haven't experimented but the following comment around line 6024 causes some
concern:
/* If the second argument is variable, we can optimize it if we are
generating 64-bit code on a machine with direct move. */
Am looking for advice on next steps here.
Am happy to simply close this problem report if that's the right thing to do.
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532 --- Comment #8 from Segher Boessenkool --- (In reply to Andrew Pinski from comment #6) > Note IIRC vec_extract came from the Cell BE C/C++ extension guide. I can't > seem to find that guide any more either :(. Try googling for "Language_Extensions_for_CBEA_2.4.pdf"? "The element that is specified by element is extracted from vector a and returned in scalar d. Depending on the size of the element, only a limited number of the least significant bits of the element index are used. Specifically for 1-, 2-, and 4-byte elements, only four, three, and two of the least significant bits are used, respectively."
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532
--- Comment #7 from Bill Schmidt ---
To be absolutely clear, code like
unsigned int get_auto_n_int ( vector unsigned int a, int n) { return
__builtin_vec_extract (a, n); }
is invalid. The second argument must be constant. This was not properly
documented in the ABI appendix, but should have been.
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532 --- Comment #6 from Andrew Pinski --- Note IIRC vec_extract came from the Cell BE C/C++ extension guide. I can't seem to find that guide any more either :(. It does matter less these days as the ABI documents this intrinsics now too.
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532 --- Comment #5 from Bill Schmidt --- To your second point, the new intrinsic programming reference under development already abandons the language about v[i], so that's covered. The next version of the ABI will drop vector API stuff (chapter 6 and Appendix A) in favor of the new document. I don't think we should change the error message, either. Even though we tolerate an out-of-range constant doesn't mean that we should advertise it on the error message. The present message is useful as is in my view. All present behavior is as I expect, so this is a bad test case. I'd say fix the test and close the issue.
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532
--- Comment #4 from kelvin at gcc dot gnu.org ---
Is this a bug or just "bad documentation"?
64-Bit ELF V2 ABI Specification says vec_extract (v, 3) is equivalent to v[3].
Then it clarifies that vec_extract (arg1, arg2) uses modular arithmetic on arg2
to determine the index position from which to extract the result. If v is
vector int, then vec_extract(vi, 10) equals vec_extract (vi, 10%4) which is
vec_extract (vi, 2).
I am familiar with the error messages reported for the test programs attached
to this problem report. The error messages are misleading and inconsistent
with actual implementation.
The following test reveals some of the current implementation behaviors.
#include
#include "altivec.h"
vector int vi;
extern void clobber ();
#ifdef ILLEGAL_CODE
//__attribute ((noinline))
int doextractbybuiltin (vector int vi, int index) {
/* Builtin expansion of vec_extract requires that index be a
compile-time constant. The generated error message says the selector
must be an integer constant in the range 0..3, which is not accurate. */
return vec_extract (vi, index);
}
#endif
//__attribute ((noinline))
int doextractbybrace (vector int vi, int index) {
return vi [index];
}
int main (int argc, char *argv)
{
vi[0] = 0x00ff00;
vi[1] = 0x01ff01;
vi[2] = 0x02ff02;
vi[3] = 0x03ff03;
printf ("a: 0x%x\n", doextractbybrace (vi, 3)); // outputs 0x3ff03
printf ("b: 0x%x\n", doextractbybrace (vi, 10)); // outputs 0
// (undefined behavior?)
// which, by the way, is not the same as vec_extract (vi, 10)
#ifdef ILLEGAL_CODE
printf ("c: 0x%x\n", doextractbybuiltin (vi, 3));
printf ("d: 0x%x\n", doextractbybuiltin (vi, 10));
#endif
printf ("e: 0x%x\n", vi[3]); // outputs 0x3ff03
printf ("f: 0x%x\n", vi[10]); // outputs 0 (undefined behavior?)
printf ("g: 0x%x\n", vec_extract(vi, 3)); // outputs 0x3ff03
printf ("h: 0x%x\n", vec_extract(vi, 10)); // outputs 0x2ff02
}
What do we want to change, if anything, about the current implementation?
My inclination:
Change the generated error message to require selector to be a constant (but
not specify that it be in the range 0..3
Change the ABI description to not claim that vec_extract (v, i) is equivalent
to v[i] and clarify that i must be a constant.
Do we want to do more?
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532 Bill Schmidt changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2019-01-30 Ever confirmed|0 |1 --- Comment #3 from Bill Schmidt --- (Confirmed. Kelvin has a patch.)
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532 Will Schmidt changed: What|Removed |Added CC||will_schmidt at vnet dot ibm.com --- Comment #2 from Will Schmidt --- Created attachment 45446 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45446&action=edit another testcase variation... Another testcase variation. I've cleaned up/removed some casts that may have been adding to the confusion earlier. This test variation exercises the vec_extract against char, short, int types. Per inspection, both char and short types appear to show incorrect results when built with -O2 for power8 or newer.
[Bug target/87532] bad results from vec_extract(unsigned char, foo) dependent upon function inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87532 --- Comment #1 from Will Schmidt --- Created attachment 44797 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=44797&action=edit simpler testcase variation Simplified the testcase a bit. comment/uncomment the noinline attribute on the get_auto_n() function to toggle pass/fail. Fails with -O2. on Power8. (probably also power9).
