* Suspected bugs in ptr_difference_const & split_address_to_core_and_offset
@ 2014-01-24 19:14 Bingfeng Mei
2014-01-28 12:55 ` Richard Biener
0 siblings, 1 reply; 2+ messages in thread
From: Bingfeng Mei @ 2014-01-24 19:14 UTC (permalink / raw)
To: gcc; +Cc: ook
Hi,
I experienced an issue in our port, which I suspected due to bugs
in ptr_difference_const & split_address_to_core_and_offset. Basically,
ptr_difference_const, called by ivopts pass, tries to evaluate
whether e1 & e2 differ by a const. In this example,
e1 is (addr_expr (mem_ref (ssa_name1, 8))
e2 is just ssa_name1.
It is obvious to me that ptr_difference_const should return true. But
it calls split_address_to_core_and_offset to split e1 to some base pointer
and offset. split_addess_to_core_and_offset in turn calls get_inner_reference
to do it. get_inner_reference cannot handle (mem_ref (ssa_name1, 8)),
it just returns the same expression back.
get_inner_reference function
case MEM_REF:
/* Hand back the decl for MEM[&decl, off]. */
if (TREE_CODE (TREE_OPERAND (exp, 0)) == ADDR_EXPR)
{
tree off = TREE_OPERAND (exp, 1);
if (!integer_zerop (off))
{
double_int boff, coff = mem_ref_offset (exp);
boff = coff.alshift (BITS_PER_UNIT == 8
? 3 : exact_log2 (BITS_PER_UNIT),
HOST_BITS_PER_DOUBLE_INT);
bit_offset += boff;
}
exp = TREE_OPERAND (TREE_OPERAND (exp, 0), 0);
}
goto done;
Then in ptr_difference_const, we get core1 as (mem_ref (ssa_name1, 8))
and toffset1 is empty. ptr_difference_const will return false as result.
There is another possible bug in ptr_difference_const. If one of toffset1
& toffset2 is true, why it returns false? The comment doesn't make sense
to me. In this example, toffset1 should be 8 and toffset2 should be empty.
No way it should return false.
else if (toffset1 || toffset2)
{
/* If only one of the offsets is non-constant, the difference cannot
be a constant. */
return false;
}
Any comment? I would like to submit a patch for it. The problem is I don't
have an reproducible example on x86 or other public targets. I ran through
the x86-64 tests and didn't hit a single case that meets this condition.
e1 is (addr_expr (mem_ref (ssa_name1, 8))
e2 is just ssa_name1.
Not sure about other targets though.
Thanks,
Bingfeng
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Suspected bugs in ptr_difference_const & split_address_to_core_and_offset
2014-01-24 19:14 Suspected bugs in ptr_difference_const & split_address_to_core_and_offset Bingfeng Mei
@ 2014-01-28 12:55 ` Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2014-01-28 12:55 UTC (permalink / raw)
To: Bingfeng Mei; +Cc: gcc, ook
On Fri, Jan 24, 2014 at 6:47 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
> Hi,
> I experienced an issue in our port, which I suspected due to bugs
> in ptr_difference_const & split_address_to_core_and_offset. Basically,
> ptr_difference_const, called by ivopts pass, tries to evaluate
> whether e1 & e2 differ by a const. In this example,
>
> e1 is (addr_expr (mem_ref (ssa_name1, 8))
> e2 is just ssa_name1.
>
> It is obvious to me that ptr_difference_const should return true. But
> it calls split_address_to_core_and_offset to split e1 to some base pointer
> and offset. split_addess_to_core_and_offset in turn calls get_inner_reference
> to do it. get_inner_reference cannot handle (mem_ref (ssa_name1, 8)),
> it just returns the same expression back.
>
> get_inner_reference function
> case MEM_REF:
> /* Hand back the decl for MEM[&decl, off]. */
> if (TREE_CODE (TREE_OPERAND (exp, 0)) == ADDR_EXPR)
> {
> tree off = TREE_OPERAND (exp, 1);
> if (!integer_zerop (off))
> {
> double_int boff, coff = mem_ref_offset (exp);
> boff = coff.alshift (BITS_PER_UNIT == 8
> ? 3 : exact_log2 (BITS_PER_UNIT),
> HOST_BITS_PER_DOUBLE_INT);
> bit_offset += boff;
> }
> exp = TREE_OPERAND (TREE_OPERAND (exp, 0), 0);
> }
> goto done;
>
> Then in ptr_difference_const, we get core1 as (mem_ref (ssa_name1, 8))
> and toffset1 is empty. ptr_difference_const will return false as result.
That's because get_inner_reference is the wrong tool to ask for
a base _address_ IMHO. In theory get_inner_reference could
return MEM[ptr, 0] of course but that requires building a new tree
which isn't the suitable thing to do here.
What you want is a get_base_address_and_constant_offset_part.
It may be as simple as wrapping get_inner_reference
to perform the final step and adjust the kind of tree it is supposed to
return.
>
> There is another possible bug in ptr_difference_const. If one of toffset1
> & toffset2 is true, why it returns false? The comment doesn't make sense
> to me. In this example, toffset1 should be 8 and toffset2 should be empty.
No, I think in this example bitpos should have the 8, not toffset. toffset
are supposed to be non-constant parts.
I think the fix belongs into split_address_to_core_and_offset, handling
MEM[X, CST], avoiding the build_fold_addr_expr_loc and adjusting
pbitpos for CST.
Richard.
> No way it should return false.
>
> else if (toffset1 || toffset2)
> {
> /* If only one of the offsets is non-constant, the difference cannot
> be a constant. */
> return false;
> }
>
>
> Any comment? I would like to submit a patch for it. The problem is I don't
> have an reproducible example on x86 or other public targets. I ran through
> the x86-64 tests and didn't hit a single case that meets this condition.
> e1 is (addr_expr (mem_ref (ssa_name1, 8))
> e2 is just ssa_name1.
> Not sure about other targets though.
>
> Thanks,
> Bingfeng
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-01-28 11:56 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-24 19:14 Suspected bugs in ptr_difference_const & split_address_to_core_and_offset Bingfeng Mei
2014-01-28 12:55 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).