Re: [PATCH] Vector to scalar casting and widening

[Tom Tromey <tromey@redhat.com>]

>>>>> "Andrew" == Andrew Burgess <aburgess@broadcom.com> writes:

Andrew> I'd like to change the way gdb handles scalar to vector
Andrew> widening.  I believe that the changes I propose will bring gdb
Andrew> expression evaluation into line with how gcc handles these
Andrew> things; this seems a good thing to me, but I'd be interested to
Andrew> hear why anyone things we should stick with the current scheme.

I agree that we should generally follow the compiler.

In this case what I would suggest is looking up the original vector
patches to gdb to see whether there is some rationale given for the
chosen behavior.

I'm in favor of this change unless that research turns up something; in
which case we should probably discuss it more.

Andrew> +static struct value *
Andrew> +vector_widen (struct value *scalar_value, struct type *vector_type)
Andrew> +{
Andrew> +  /* Widen the scalar to a vector.  */
Andrew> +  struct type *eltype, *scalar_type;
Andrew> +  struct value *val, *elval;
Andrew> +  LONGEST low_bound, high_bound;
Andrew> +  int i;
Andrew> +
Andrew> +  gdb_assert (TYPE_CODE (check_typedef (vector_type)) == TYPE_CODE_ARRAY);
Andrew> +  gdb_assert (TYPE_VECTOR (vector_type));
Andrew> +
Andrew> +  if (!get_array_bounds (vector_type, &low_bound, &high_bound))
Andrew> +    error (_("Could not determine the vector bounds"));
Andrew> +
Andrew> +  eltype = check_typedef (TYPE_TARGET_TYPE (vector_type));
Andrew> +  elval = value_cast (eltype, scalar_value);

I think check_typedef is being applied inconsistently here.
I'd suggest starting the function with a call to the macro:

    CHECK_TYPEDEF (vector_type);

then you can drop check_typedef from the first assert.

Andrew> +  else if (code1 == TYPE_CODE_ARRAY && TYPE_VECTOR (type)
Andrew> +	   && code2 == TYPE_CODE_ARRAY && TYPE_VECTOR (type2)
Andrew> +	   && TYPE_LENGTH (type) != TYPE_LENGTH (type2))
Andrew> +    error (_("can't convert between vector values of different size"));

I didn't see a test case for this error.
Could you add one?

Andrew> +++ b/gdb/testsuite/gdb.base/gnu_vector.c
Andrew> @@ -31,6 +31,7 @@ int ia = 2;
Andrew>  int ib = 1;
Andrew>  float fa = 2;
Andrew>  float fb = 1;
Andrew> +long long lla = 0x0000000100000001ll;

Here I don't think we can assume that lla has a particular size, can we?
But since this is a gcc-specific test, I think you can work around it by
using the 'mode' attribute to pick a particular size.

Andrew> +++ b/gdb/testsuite/gdb.python/py-type.c
Andrew> @@ -15,6 +15,8 @@
Andrew>     You should have received a copy of the GNU General Public License
Andrew>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
Andrew> +#include <stdint.h>

I'm mildly concerned that this will mean that we can't run this test on
some platform.  But only mildly, I'm inclined to let it go.

Tom