> Am 08.06.2026 um 17:29 schrieb Andrew MacLeod <[email protected]>:
>
> THis PR is open against GC 14,15, and 16 as well. Should I apply it to all
> those branches, or selectively which ones?
>
> Should be a conservatively harmless fix...
All of them. Note the 15 branch is currently frozen for the 15.3 release.
Richard
> Andrew
>
>
>> On 6/5/26 13:12, Andrew MacLeod wrote:
>>
>>> On 6/5/26 11:35, Jeffrey Law wrote:
>>>
>>>
>>> On 6/5/2026 8:51 AM, Andrew MacLeod wrote:
>>>> If the condition leading to a __builtin_unreachable involves more than one
>>>> SSA name, it is unsafe to assign a global range to an SSA name even if all
>>>> current uses are valid
>>>>
>>>> if (a == 0)
>>>> return
>>>> <...>
>>>> if (a == b)
>>>> = b
>>>> else
>>>> __builtin_unreachable ()
>>>>
>>>> DOM thinks all uses of b can be given the global value of [1, +INF]
>>>> because a value of 0 will hit the __builtin_unreachable() call.
>>>> It does not take into account that b has a relation with a, and if a later
>>>> pass moves these conditions around, that assumption may not be valid any
>>>> longer.
>>>>
>>>> VRP refuses to attempt early resolution of builtin_unreachable () calls if
>>>> the expression leading the builtin_unreachable() contains 2 SSA names as
>>>> the relation introduced makes it unsafe.
>>>>
>>>> This patch give DOM the same early exit... check if there are 2 SSA names
>>>> on the condition and not try to assign a global range if so.
>>>>
>>>> Bootstraps on x86_64-pc-linux-gnu and no regressions. (Presumably.. there
>>>> appear to be spurious avx testcases failing that seem unrelated to
>>>> whatever patch I apply)
>>> My big question is do we have a deeper problem here. It sounds like
>>> unswitch swapped two statements which invalidated a global range that DOM
>>> had recorded? Doesn't that really point to a problem with unswitch in that
>>> the unswitch transformation invalidated a global range without clearing it?
>>> Are there any other places where we could record a global range based on
>>> properties that unswitch (or another pass) might change?
>>>
>>>
>> There's a long discussion in the PR. I cannot comment on the safety of what
>> unswitching does in general, but I can comment on the safety of assigning
>> the global.
>>
>> When I rewrote the __builtin_unreachable (), I found it was generally unsafe
>> to export global values early unless *All* values generated by the edge
>> could be replaced with the value generated by the edge.
>>
>> So its safe for
>> if (a == 0) __builtin_unreachable()
>> to set the global range for a to [1, +INF] if this dominates all uses of a.
>>
>> Given
>> if (a == b) __builtin_unreachable ()
>> Its only safe to to set global ranges if both a *and* b can be set... ie,
>> its unsafe to set 'b' to some range, and not set 'a'. There is a relation
>> introduced between them, and if the global value do not reflect the
>> relation, we can get into trouble later.
>>
>> In this testcase, we can not propagate both values to be [1, +INF], and that
>> equality introduced ended up causing the issue when some code was moved
>> around.
>>
>> The need to assign global values early like this is far less important than
>> it once was.. That same information is available from ranger everywhere,
>> and globals should really only be important once we are set to drop from
>> SSA. EVRP and VRP1 will do some early glpobal assignments, but only proven
>> safe ones. VRP2 drops the builtin assumes and does what it can for globals
>> safely at that point. I don't think we should be applying contextual
>> derived information to global values before then unless we are 100% ultra
>> sure its safe.
>>
>> My 2 cents.
>> Andrew
>>
>> PS, and clearing global information is not a good idea if at all avoidable..
>> that information may have come from IPA or elsewhere and not be
>> reproducible. As long as we never lie about a global range, we shouldn't
>> have any problems moving code around :-)
>>
>>
>
