[Bug tree-optimization/114769] [14 Regression] Suspicious code in vect_recog_sad_pattern() since r14-1832
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114769 Tamar Christina changed: What|Removed |Added Resolution|--- |FIXED Status|ASSIGNED|RESOLVED --- Comment #5 from Tamar Christina --- (In reply to Feng Xue from comment #3) > When half_type is null, and call chain would be: > > vect_supportable_direct_optab_p (vinfo, sum_type, SAD_EXPR, NULL, ...) > -> get_vectype_for_scalar_type (vinfo, NULL) > -> get_related_vectype_for_scalar_type (vinfo, /*scalar_type= */NULL, > ...) > >if ((!INTEGRAL_TYPE_P (scalar_type) >&& !POINTER_TYPE_P (scalar_type) >&& !SCALAR_FLOAT_TYPE_P (scalar_type)) > > Here will be a segfault. Sure, I wasn't able to trigger that even disabling the optab. In any case this version doesn't get there anymore. So fixed, thanks for the report!
[Bug tree-optimization/114769] [14 Regression] Suspicious code in vect_recog_sad_pattern() since r14-1832
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114769 --- Comment #4 from GCC Commits --- The master branch has been updated by Tamar Christina : https://gcc.gnu.org/g:1216460e7023cd8ec49933866107417c70e933c9 commit r14-10040-g1216460e7023cd8ec49933866107417c70e933c9 Author: Tamar Christina Date: Fri Apr 19 15:22:13 2024 +0100 middle-end: refactory vect_recog_absolute_difference to simplify flow [PR114769] Hi All, As the reporter in PR114769 points out the control flow for the abd detection is hard to follow. This is because vect_recog_absolute_difference has two different ways it can return true. 1. It can return true when the widening operation is matched, in which case unprom is set, half_type is not NULL and diff_stmt is not set. 2. It can return true when the widening operation is not matched, but the stmt being checked is a minus. In this case unprom is not set, half_type is set to NULL and diff_stmt is set. This because to get to diff_stmt you have to dig through the abs statement and any possible promotions. This however leads to complicated uses of the function at the call sites as the exact semantic needs to be known to use it safely. vect_recog_absolute_difference has two callers: 1. vect_recog_sad_pattern where if you return true with unprom not set, then *half_type will be NULL. The call to vect_supportable_direct_optab_p will always reject it since there's no vector mode for NULL. Note that if looking at the dump files, the convention in the dump files have always been that we first indicate that a pattern could possibly be recognize and then check that it's supported. This change somewhat incorrectly makes the diagnostic message get printed for "invalid" patterns. 2. vect_recog_abd_pattern, where if half_type is NULL, it then uses diff_stmt to set them. This refactors the code, it now only has 1 success condition, and diff_stmt is always set to the minus statement in the abs if there is one. The function now only returns success if the widening minus is found, in which case unprom and half_type set. This then leaves it up to the caller to decide if they want to do anything with diff_stmt. Thanks, Tamar gcc/ChangeLog: PR tree-optimization/114769 * tree-vect-patterns.cc: (vect_recog_absolute_difference): Have only one success condition. (vect_recog_abd_pattern): Handle further checks if vect_recog_absolute_difference fails.
[Bug tree-optimization/114769] [14 Regression] Suspicious code in vect_recog_sad_pattern() since r14-1832
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114769 --- Comment #3 from Feng Xue --- When half_type is null, and call chain would be: vect_supportable_direct_optab_p (vinfo, sum_type, SAD_EXPR, NULL, ...) -> get_vectype_for_scalar_type (vinfo, NULL) -> get_related_vectype_for_scalar_type (vinfo, /*scalar_type= */NULL, ...) if ((!INTEGRAL_TYPE_P (scalar_type) && !POINTER_TYPE_P (scalar_type) && !SCALAR_FLOAT_TYPE_P (scalar_type)) Here will be a segfault.
[Bug tree-optimization/114769] [14 Regression] Suspicious code in vect_recog_sad_pattern() since r14-1832
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114769 --- Comment #2 from Tamar Christina --- I believe this is safe, but the interface is definitely not the cleanest. vect_recog_absolute_difference has two callers: 1. vect_recog_sad_pattern where if you return true with unprom not set, then *half_type will be NULL. The call to vect_supportable_direct_optab_p will always reject it since there's no vector mode for NULL. Note that if looking at the dump files, the convention in the dump files have always been that we first indicate that a pattern could possibly be recognize and then check that it's supported. This change somewhat incorrectly makes the diagnostic message get printed for "invalid" patterns. 2. vect_recog_abd_pattern, where if half_type is NULL, it then uses diff_stmt to set them. So while the note in the dump file is misleading, the code is safe. I will however slightly refactor it to prevent the confusion.
[Bug tree-optimization/114769] [14 Regression] Suspicious code in vect_recog_sad_pattern() since r14-1832
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114769 Richard Biener changed: What|Removed |Added Ever confirmed|0 |1 Last reconfirmed||2024-04-19 Status|UNCONFIRMED |ASSIGNED Assignee|unassigned at gcc dot gnu.org |rguenth at gcc dot gnu.org
[Bug tree-optimization/114769] [14 Regression] Suspicious code in vect_recog_sad_pattern() since r14-1832
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114769 Jakub Jelinek changed: What|Removed |Added Summary|[14 Regression] Suspicious |[14 Regression] Suspicious |code in |code in |vect_recog_sad_pattern()|vect_recog_sad_pattern() ||since r14-1832 CC||jakub at gcc dot gnu.org --- Comment #1 from Jakub Jelinek --- Added in r14-1832-g710b8dec61a73cb
