On Wed, Sep 25, 2013 at 1:37 PM, Yufeng Zhang <yufeng.zh...@arm.com> wrote:
> Hello,
>
> Please find the updated version of the patch in the attachment. It has
> addressed the previous comments and also included some changes in order to
> pass the bootstrapping on x86_64.
>
> It's also passed the regtest on arm-none-eabi and aarch64-none-elf.
>
> It will also fix the test failure as reported here:
> http://gcc.gnu.org/ml/gcc-patches/2013-09/msg01317.html
>
> OK for the trunk?
+ where n is a 32-bit unsigned int and pointer are 64-bit long. In this
+ case, the gimple for (n - 1) is:
+
+ _2 = n_1(D) + 4294967295; // 0xFFFFFFFF
+
+ and it is wrong to multiply the large constant by 4 in the 64-bit space. */
+
+static bool
+safe_to_multiply_p (tree type, double_int cst)
+{
+ if (TYPE_UNSIGNED (type)
+ && ! double_int_fits_to_tree_p (signed_type_for (type), cst))
+ return false;
+
+ return true;
+}
This looks wrong. The only relevant check is as whether the
multiplication overflows the original type as you miss the implicit
truncation that happens. Which is something you don't know
unless you know the value. It definitely isn't a property of a type
and a constant but the property of two constants and a type.
Or the predicate has a wrong name.
The use of get_unwidened in this core routine looks like this is
all happening in the wrong place and we should have picked up
another candidate for this instead? I'm sure Bill will know more here.
Richard.
> Thanks,
> Yufeng
>
>
> gcc/
>
> * gimple-ssa-strength-reduction.c (safe_to_multiply_p): New
> function.
> (backtrace_base_for_ref): Call get_unwidened, check 'base_in'
> again and set unwidend_p with true; call safe_to_multiply_p to avoid
> unsafe unwidened cases.
>
> gcc/testsuite/
>
> * gcc.dg/tree-ssa/slsr-40.c: New test.
>
>
>
>
> On 09/11/13 13:39, Bill Schmidt wrote:
>>
>> On Wed, 2013-09-11 at 10:32 +0200, Richard Biener wrote:
>>>
>>> On Tue, Sep 10, 2013 at 5:53 PM, Yufeng Zhang<yufeng.zh...@arm.com>
>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> Following Bin's patch in
>>>> http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00695.html, this patch
>>>> tweaks
>>>> backtrace_base_for_ref () to strip of any widening conversion after the
>>>> first TREE_CODE check fails. Without this patch, the test
>>>> (gcc.dg/tree-ssa/slsr-39.c) in Bin's patch will fail on AArch64, as
>>>> backtrace_base_for_ref () will stop if not seeing an ssa_name since the
>>>> tree
>>>> code can be nop_expr instead.
>>>>
>>>> Regtested on arm and aarch64; still bootstrapping x86_64.
>>>>
>>>> OK for the trunk if the x86_64 bootstrap succeeds?
>>>
>>>
>>> Please add a testcase.
>>
>>
>> Also, the comment "Strip of" should read "Strip off". Otherwise I have
>> no comments.
>>
>> Thanks,
>> Bill
>>
>>>
>>> Richard.
