Re: [range-ops] patch 01/04: types for VR_UNDEFINED and VR_VARYING

[Jeff Law <law at redhat dot 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