On Mon, Feb 2, 2015 at 11:37 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Mon, Feb 02, 2015 at 02:51:43PM -0800, Andrew Pinski wrote:
>> While trying to build the GCC 5 with GCC 5, I ran into an ICE when
>> building libcpp at -O0. The problem is the C++ front-end was not
>> folding sizeof(a)/sizeof(a[0]) when passed to a function at -O0. The
>> C++ front-end keeps around sizeof until the gimplifier and there is no
>> way to fold the expressions that involve them. So to work around the
>> issue we need to change __builtin_aarch64_im_lane_boundsi to accept an
>> extra argument and change the first two arguments to size_t type so we
>> don't get an extra cast there and do the division inside the compiler
>> itself.
>
> Relying on anything being folded at -O0 when the language does not guarantee
> it is going to be more and more of a problem. So I think your patch is
> reasonable (of course, I'll defer this to target maintainers).
>
>> + rtx totalsize = expand_normal (CALL_EXPR_ARG (exp, 0));
>> + rtx elementsize = expand_normal (CALL_EXPR_ARG (exp, 1));
>> + if (CONST_INT_P (totalsize) && CONST_INT_P (elementsize))
>> + {
>> + rtx lane_idx = expand_normal (CALL_EXPR_ARG (exp, 2));
>> + if (CONST_INT_P (lane_idx))
>> + aarch64_simd_lane_bounds (lane_idx, 0, UINTVAL (totalsize)/UINTVAL
>> (elementsize), exp);
>
> Too long line? Also, missing spaces around / . And, ICE if
> somebody uses __builtin_aarch64_im_lane_boundsi (4, 0, 0);
> So you need to check and complain for zero elementsize too.
Good points, I don't know why I missed this.
>
>> + else
>> + error ("%Klane index must be a constant immediate", exp);
>> + }
>> else
>> - error ("%Klane index must be a constant immediate", exp);
>> + sorry ("%Ktotal size and element size must be a constant immediate",
>> exp);
>
> But why sorry? If you say the builtin requires constant arguments, then it
> is not sorry, but error, it is not an unimplemented feature.
Because I originally thought that would be better than error but now
thinking this over, I was incorrect, it should be an error.
I will add a testcase for the __builtin_aarch64_im_lane_boundsi (4, 0,
0) case and retest the patch.
Thanks,
Andrew
>
> Jakub
