This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Scalar vector binary operation
- From: "Joseph S. Myers" <joseph at codesourcery dot com>
- To: Artem Shinkarov <artyom dot shinkaroff at gmail dot com>
- Cc: Richard Guenther <richard dot guenther at gmail dot com>, gcc-patches at gcc dot gnu dot org
- Date: Fri, 3 Dec 2010 18:00:18 +0000 (UTC)
- Subject: Re: Scalar vector binary operation
- References: <AANLkTi=UguCY9pZBg5N_K-NHcVSEA=8UTLhtWPjVtdiO@mail.gmail.com> <AANLkTinr4eu1PwGz1P7zczGq7E3A5M77Y0==wrazF5NQ@mail.gmail.com> <AANLkTikPoL+wSw2BrUxzQw7h_NenRT0ATyQL6SdxVoJU@mail.gmail.com> <Pine.LNX.4.64.1011061735220.21929@digraph.polyomino.org.uk> <AANLkTi=Aivzx=XVig+dQ+tEqJtrFGhVD0VCkAmp4nLw-@mail.gmail.com> <Pine.LNX.4.64.1011122118050.1893@digraph.polyomino.org.uk> <AANLkTimnBQpigzjjh=tjHEWZsRhqOU-P0HgiQ7cLbjzX@mail.gmail.com> <Pine.LNX.4.64.1011161633490.14051@digraph.polyomino.org.uk> <AANLkTim1vizSgSsO9C37=8fvkTOL-wufGaRKwcF0ctGj@mail.gmail.com> <Pine.LNX.4.64.1011192158210.9350@digraph.polyomino.org.uk> <AANLkTi=E967vaGgbD6dvyhkZuw=Bcb2A+ZJ7XJV6JWje@mail.gmail.com>
On Mon, 22 Nov 2010, Artem Shinkarov wrote:
> + case stv_firstarg:
> + {
> + tree sc = save_expr (op0);
> + sc = convert (TREE_TYPE (type1), sc);
> + op0 = build_vector_from_val (type1, sc);
> + orig_type0 = type0 = TREE_TYPE (op0);
> + code0 = TREE_CODE (type0);
> + converted = 1;
> + break;
> + }
> + case stv_secondarg:
> + {
> + tree sc = save_expr (op1);
> + sc = convert (TREE_TYPE (type0), sc);
> + op1 = build_vector_from_val (type0, sc);
> + orig_type1 = type1 = TREE_TYPE (op1);
> + code1 = TREE_CODE (type1);
> + converted = 1;
> + break;
> + }
(Watch your indentation; here and elsewhere in the patch you add new code
indented by eight or more spaces when TABs should be used. You also have
a bogus patch hunk in conversion_warning that changes a TAB to spaces and
does nothing else.)
This code has the problem that it can call save_expr on something
containing a C_MAYBE_CONST_EXPR without passing it through c_fully_fold
first. Simply using c_save_expr instead of save_expr isn't enough because
that can result in a C_MAYBE_CONST_EXPR inside a vector. Instead you want
something like
+ case stv_firstarg:
+ {
+ bool maybe_const = true;
+ tree sc;
+ sc = c_fully_fold (op0, false, &maybe_const);
+ sc = save_expr (sc);
+ sc = convert (TREE_TYPE (type1), sc);
+ op0 = build_vector_from_val (type1, sc);
+ if (!maybe_const)
+ op0 = c_wrap_maybe_const (op0, true);
+ orig_type0 = type0 = TREE_TYPE (op0);
+ code0 = TREE_CODE (type0);
+ converted = 1;
+ break;
+ }
and similarly for the second case. Testcase (that ICEs with your
original patch):
int f(void);
unsigned int g(void);
unsigned int h;
typedef unsigned int vec __attribute__((vector_size(16)));
vec i;
vec fv1(void) { return i + (h ? f() : g()); }
vec fv2(void) { return (h ? f() : g()) + i; }
The patch is OK modified as indicated and with that testcase added to the
testsuite (presuming it still passes bootstrap with no regressions).
--
Joseph S. Myers
joseph@codesourcery.com