From: Jeff Law <law@redhat.com>
To: Aldy Hernandez <aldyh@redhat.com>,
Richard Biener <richard.guenther@gmail.com>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>,
Andrew MacLeod <amacleod@redhat.com>
Subject: Re: [range-ops] patch 01/04: types for VR_UNDEFINED and VR_VARYING
Date: Wed, 03 Jul 2019 22:40:00 -0000 [thread overview]
Message-ID: <696e819a-57a3-5365-6834-f9be8aefbe15@redhat.com> (raw)
In-Reply-To: <982168fa-fc1e-4432-9e15-6ed2cf06195a@redhat.com>
On 7/3/19 3:46 AM, Aldy Hernandez wrote:
>
>
> On 7/2/19 4:17 PM, Jeff Law wrote:
>> On 7/1/19 2:52 AM, Aldy Hernandez wrote:
>>> As discussed before, this enforces types on undefined and varying, which
>>> makes everything more regular, and removes some special casing
>>> throughout range handling.
>>>
>>> The min/max fields will contain TYPE_MIN_VALUE and TYPE_MAX_VALUE, which
>>> will make it easy to get at the bounds of a range later on. Since
>>> pointers don't have TYPE_MIN/MAX_VALUE, we are using build_zero_cst()
>>> and wide_int_to_tree(wi::max_value(precision)), for consistency.
>>> UNDEFINED is set similarly, though nobody should ever ask for anything
>>> except type() from it.  That is, no one should be asking for the bounds.
>>>
>>> There is one wrinkle, ipa-cp creates VR_VARYING ranges of floats,
>>> presumably to pass around state?? This causes value_range_base::type()
>>> and others to fail, even though I haven't actually seen a case of
>>> someone actually trying to set a VR_RANGE of a float. For now, I've
>>> built a NOP_EXPR wrapper so type() works correctly. The correct
>>> approach would probably be to avoid creating these varying/undefined
>>> ranges in ipa-cp, but I don't have enough ipa-cp-foo to do so.
>>> Suggestions welcome, if you don't like special casing this for ipa-cp.
>>> Or perhaps as a follow up.
>> No idea why we create ranges of floats from ipa-cp. I presume it's
>> coming from propagate_vr_across_jump_function? Or somewhere else?
>
> I believe it was ipcp_vr_lattice::set_to_bottom, while changing an
> UNDEFINED to VARYING. IMO, we shouldn't even have created UNDEFINED
> ranges of floats. It's not like we can do anything with float ranges.
Note that propagate_vr_across_jump_function calls set_to_bottom ;-)
I zero'd in on that one because it did so when presented with something
that wasn't an INTEGRAL_TYPE_P and wasn't POINTER_TYPE_P.
I think digging into where these are coming from would be advisable.
Hell, if you've got types, abort the first time we try to create a range
for something that isn't an integer/pointer, then walk backwards.
I wouldn't be surprised if we find just a couple sites that are
responsible for these problems in ipa-cp.
>>> +/* Allocate a new range from the obstack and set it to VARYING for
>>> TYPE. */
>>> +inline value_range *
>>> +type_range_cache::new_varying (tree type)
>>> +{
>>> + /* Allocate memory. */
>>> +Â void *p = XOBNEW (&m_range_obj, value_range);
>>> + /* Invoke the constructors on the memory using placement new. */
>>> +Â value_range *new_p = new (p) value_range ();
>>> + /* Initialize it to varying. */
>>> +Â new_p->set_varying (type);
>>> +Â return new_p;
>>> +}
>> So is placement new C++98/C++03 or C++11?
>>
>> If the former then we can use it, if the latter we probably can't since
>> we haven't stepped forward to C++11.
>
> Google isn't cooperating here to narrow the specific C++ version, but
> I'm seeing some very old references to placement new, from the mid to
> the late 1990s.
>
> FWIW, the above snippet shows no warnings when compiled with -std=c++-98
> -Wall.
Given it compiles in C++-98 mode, let's consider it a non-issue.
jeff
next prev parent reply other threads:[~2019-07-03 22:38 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-01 8:52 Aldy Hernandez
2019-07-02 20:17 ` Jeff Law
2019-07-03 9:46 ` Aldy Hernandez
2019-07-03 22:40 ` Jeff Law [this message]
2019-07-03 8:28 ` Richard Biener
2019-07-03 9:19 ` Aldy Hernandez
2019-07-03 11:08 ` Richard Biener
2019-07-03 12:23 ` Aldy Hernandez
2019-07-03 12:30 ` Aldy Hernandez
2019-07-04 10:34 ` Richard Biener
2019-07-09 7:49 ` Aldy Hernandez
2019-07-09 9:57 ` Richard Biener
2019-07-16 19:56 ` Andrew MacLeod
2019-07-22 15:38 ` Aldy Hernandez
2019-07-23 0:19 ` Jeff Law
2019-07-23 9:45 ` Richard Biener
2019-07-24 16:09 ` Jeff Law
2019-07-24 17:06 ` Richard Biener
2019-07-24 18:33 ` Jeff Law
2019-07-24 18:38 ` Richard Biener
2019-07-26 3:41 ` Jeff Law
2019-07-26 14:52 ` Andrew MacLeod
2019-07-30 9:02 ` Richard Biener
2019-07-30 15:16 ` Andrew MacLeod
2019-07-31 8:37 ` Richard Biener
[not found] ` <78846d0a-354e-b73a-6e15-123752038fb2@redhat.com>
2019-08-01 14:11 ` Richard Biener
2019-08-01 16:35 ` Andrew MacLeod
2019-07-26 4:34 ` Jeff Law
2019-07-24 20:52 ` Andrew MacLeod
2019-07-25 9:13 ` Richard Biener
2019-07-23 9:34 ` Richard Biener
2019-07-23 23:05 ` Andrew MacLeod
2019-07-24 13:28 ` Richard Biener
2019-07-24 18:07 ` Andrew MacLeod
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=696e819a-57a3-5365-6834-f9be8aefbe15@redhat.com \
--to=law@redhat.com \
--cc=aldyh@redhat.com \
--cc=amacleod@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=richard.guenther@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).