[llvm-commits] [PATCH] Avoid overflow in scheduling

[Evan Cheng]

On Sep 27, 2009, at 9:17 PM, Reid Kleckner wrote:

> I added assertions to catch overflow, and they fired when I ran it on
> the input.  Should I really keep these in?  I feel like it's always
> assumed that there is no integer overflow in a program, and that any
> overflow is an error.  These assertions just clutter the code if we
> move from shorts to ints.  If you want to stick with shorts, then I
> agree, because there is actually a risk of the overflow, they should
> probably stay in.

I think the assertion should stay because the bug is very hard to  
track down. If the field is changed to ints it should be adjusted  
accordingly.

>
> I think the characteristic of this input is that we have 35K BB's in
> the function, and each line has a guard that creates a basic block
> edge to a single bailing block.  When it's time to do scheduling, that
> manifests as more than SHRT_MAX predecessors, causing the overflow in
> NumSuccs.
>
> Talking with jyasskin and nlewycky, we think that if LLVM wants the
> language restriction that there be no more than SHRT_MAX incoming
> edges to a basic block, that's fine, but the verifier should catch it,
> because we run the verifier on our code.  Otherwise, scheduling
> probably needs to support more predecessors.
>
> One other option is to change the relevant shorts from signed to
> unsigned, which I think would solve our problems.  However, if we
> simply doubled the input size, it would break again.  Using ints
> should only add 8 bytes to the entire class, which has plenty of
> pointer fields and two SmallVectors.  The size increase should only be
> a small percentage.

Even if the cost is small, I am still hesitant to grow it for the sake  
of the some extreme test. It almost feel like the type should be  
configurable for different uses.

Evan

>
> For the curious, this is the bad .ll file linked to in the bug:
> http://web.mit.edu/rnk/www/bad_ir_small.ll.bz2 (2.5 MB)
>
> Reid
>
> On Sun, Sep 27, 2009 at 1:19 AM, Evan Cheng <evan.cheng at apple.com>  
> wrote:
>> Also, could you first add assertions to catch the overflow? Thanks.
>>
>> Evan
>>
>> On Sep 26, 2009, at 3:36 PM, Evan Cheng wrote:
>>
>>> I am concerned this can increase memory usage though for all  
>>> "normal"
>>> usage cases though. It can a real issue for embedded hosts.
>>>
>>> Evan
>>>
>>> On Sep 26, 2009, at 1:19 PM, Reid Kleckner wrote:
>>>
>>>> Hey all,
>>>>
>>>> One of our nasty regression test cases generates a ginormous (4  
>>>> MB of
>>>> bitcode) function that overflows a short integer in some  
>>>> instruction
>>>> scheduling code.  Is the attached fix OK?  If so, I'll commit it.
>>>>
>>>> This fixes PR4401.
>>>>
>>>> Thanks,
>>>> Reid
>>>> <isched- 
>>>> overflow.diff>_______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>