On Mon, 24 Jun 2019 at 22:02, Martin Jambor <mjam...@suse.cz> wrote:
>
> Hi,
>
> in August 2016 Prathamesh implemented inter-procedural propagation of
> known non-zero bits on integers. In August that same year he then also
> added the ability to track it for pointer, replacing separate alignment
> tracking.
>
> However, we still have an assert in ipcp_bits_lattice::meet_with from
> the first commit that checks that any binary operation coming from an
> arithmetic jump function is performed on integers. Martin discovered
> that when you compile chromium and stars are aligned correctly, you can
> end up evaluating a pointer expression MAX_EXPR (param, 0) there which
> trips the assert.
>
> Unless Prathamesh can remember a reason why the assert is important, I
> believe the correct thing is just to remove it. In the case of this
> MAX_EXPR, it will end up being evaluated in bit_value_binop which cannot
> handle it and so we will end up with a BOTTOM lattice in the end. In
> the general case, bit_value_binop operates on widest_ints and so shoud
> have no problem dealing with pointers.
Hi,
I think the assert should have been removed after adding pointer
alignment propagation.
Sorry about that -:(
Thanks,
Prathamesh
>
> I'm bootstrapping and testing the following patch to satisfy the rules
> but since it only removes an assert and adds a testcase that I checked,
> I do not expect any problems. Is it OK for trunk, GCC 9 and 8?
>
> Thanks,
>
> Martin
>
>
>
>
> 2019-06-24 Martin Jambor <mjam...@suse.cz>
>
> PR ipa/90939
> * ipa-cp.c (ipcp_bits_lattice::meet_with): Remove assert.
>
> testsuite/
> * g++.dg/lto/pr90939_[01].C: New test.
> ---
> gcc/ipa-cp.c | 1 -
> gcc/testsuite/g++.dg/lto/pr90939_0.C | 64 ++++++++++++++++++++++++++++
> gcc/testsuite/g++.dg/lto/pr90939_1.C | 45 +++++++++++++++++++
> 3 files changed, 109 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/g++.dg/lto/pr90939_0.C
> create mode 100644 gcc/testsuite/g++.dg/lto/pr90939_1.C
>
> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index d3a88756a91..69c00a9c5a5 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
> @@ -1085,7 +1085,6 @@ ipcp_bits_lattice::meet_with (ipcp_bits_lattice& other,
> unsigned precision,
> if (TREE_CODE_CLASS (code) == tcc_binary)
> {
> tree type = TREE_TYPE (operand);
> - gcc_assert (INTEGRAL_TYPE_P (type));
> widest_int o_value, o_mask;
> get_value_and_mask (operand, &o_value, &o_mask);
>
> diff --git a/gcc/testsuite/g++.dg/lto/pr90939_0.C
> b/gcc/testsuite/g++.dg/lto/pr90939_0.C
> new file mode 100644
> index 00000000000..8987c348015
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lto/pr90939_0.C
> @@ -0,0 +1,64 @@
> +// PR ipa/90939
> +// { dg-lto-do link }
> +// { dg-lto-options { { -flto -O3 } } }
> +
> +
> +typedef char uint8_t;
> +template <class T> class A {
> +public:
> + A(T *);
> +};
> +template <typename Derived, typename Base> const Derived &To(Base &p1) {
> + return static_cast<const Derived &>(p1);
> +}
> +class H;
> +template <typename, typename Base> const H *To(Base *p1) {
> + return p1 ? &To<H>(*p1) : nullptr;
> +}
> +enum TextDirection : uint8_t;
> +enum WritingMode : unsigned;
> +class B {
> +public:
> + WritingMode m_fn1();
> +};
> +class C {
> +public:
> + int &m_fn2();
> +};
> +class D { double d;};
> +class H : public D {};
> +class F {
> +public:
> + F(C, A<const int>, B *, WritingMode, TextDirection);
> +};
> +
> +class G {
> +public:
> + C NGLayoutAlgorithm_node;
> + B NGLayoutAlgorithm_space;
> + TextDirection NGLayoutAlgorithm_direction;
> + H NGLayoutAlgorithm_break_token;
> + G(A<const int> p1) __attribute__((noinline))
> + : break_token_(&NGLayoutAlgorithm_break_token),
> + container_builder_(NGLayoutAlgorithm_node, p1,
> &NGLayoutAlgorithm_space,
> + NGLayoutAlgorithm_space.m_fn1(),
> + NGLayoutAlgorithm_direction) {}
> + G(C p1, const H *) : G(&p1.m_fn2()) {}
> + A<H> break_token_;
> + F container_builder_;
> +};
> +
> +class I : G {
> +public:
> + I(const D *) __attribute__((noinline));
> +};
> +C a;
> +I::I(const D *p1) : G(a, To<H>(p1)) {}
> +
> +D gd[10];
> +
> +int main (int argc, char *argv[])
> +{
> + I i(&(gd[argc%2]));
> + return 0;
> +}
> diff --git a/gcc/testsuite/g++.dg/lto/pr90939_1.C
> b/gcc/testsuite/g++.dg/lto/pr90939_1.C
> new file mode 100644
> index 00000000000..9add89494d7
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lto/pr90939_1.C
> @@ -0,0 +1,45 @@
> +typedef char uint8_t;
> +template <class T> class A {
> +public:
> + A(T *);
> +};
> +
> +enum TextDirection : uint8_t;
> +enum WritingMode : unsigned;
> +class B {
> +public:
> + WritingMode m_fn1();
> +};
> +class C {
> +public:
> + int &m_fn2();
> +};
> +
> +class F {
> +public:
> + F(C, A<const int>, B *, WritingMode, TextDirection);
> +};
> +class D { double d;};
> +class H : public D {};
> +
> +
> +
> +template <class T> A<T>::A(T*) {}
> +
> +template class A<H>;
> +template class A<int const>;
> +
> +WritingMode __attribute__((noipa))
> +B::m_fn1()
> +{
> + return (WritingMode) 0;
> +}
> +
> +int gi;
> +int & __attribute__((noipa))
> +C::m_fn2 ()
> +{
> + return gi;
> +}
> +
> +__attribute__((noipa)) F::F(C, A<const int>, B *, WritingMode,
> TextDirection) {}
> --
> 2.21.0
>
