On Mon, May 23, 2016 at 3:16 PM, Richard Biener
<richard.guent...@gmail.com> wrote:
> On Mon, May 23, 2016 at 3:23 PM, Bin Cheng <bin.ch...@arm.com> wrote:
>> Hi,
>> When working on PR69710, I ran into this latent bug in which alignment
>> information is wrongly updated for pointer variables. It results in memory
>> exceptions on x86_64 after patch for PR69710. Scenario is that
>> copy_ref_info tries to update base's alignment in TARGET_MEM_REF[base +
>> index << step]. But case with NULL TMR_STEP (which implies the step is 1)
>> is not handled here. This patch fixes the bug by simply checking NULL
>> TMR_STEP. The conditions about TMR_STEP could be relaxed if TMR_INDEX is an
>> induction variable which has aligned initial value and step. But that needs
>> non-trivial code refactoring since copy_ref_info is uses by different parts
>> of compiler.
>>
>> Bootstrap and test on x86_64. Is it OK?
>
> I think it is ok but having !TMR_STEP when TMR_INDEX is non-NULL is
> IMHO bad and we should enforce
> that this doesn't happen in TMR verification in
> tree-cfg.c:verify_types_in_gimple_reference. I also notice
> that if TMR_INDEX is NULL then TMR_STEP being NULL shouldn't block
Yeah, patch updated slightly to handle this case.
Thanks,
bin
> alignment transfer (in that case
> TMR_INDEX2 is likely non-NULL though).
>
> Thanks,
> Richard.
>
>> Thanks,
>> bin
>>
>> 2016-05-20 Bin Cheng <bin.ch...@arm.com>
>>
>> * tree-ssa-address.c (copy_ref_info): Check null TMR_STEP.
diff --git a/gcc/tree-ssa-address.c b/gcc/tree-ssa-address.c
index 9e49f3d..b04545c 100644
--- a/gcc/tree-ssa-address.c
+++ b/gcc/tree-ssa-address.c
@@ -877,6 +877,10 @@ copy_ref_info (tree new_ref, tree old_ref)
&& TREE_CODE (old_ref) == MEM_REF
&& !(TREE_CODE (new_ref) == TARGET_MEM_REF
&& (TMR_INDEX2 (new_ref)
+ /* TODO: Below conditions can be relaxed if TMR_INDEX
+ is an indcution variable and its initial value and
+ step are aligned. */
+ || (TMR_INDEX (new_ref) && !TMR_STEP (new_ref))
|| (TMR_STEP (new_ref)
&& (TREE_INT_CST_LOW (TMR_STEP (new_ref))
< align)))))
