[llvm-commits] [PATCH] Avoid overflow in scheduling
Evan Cheng
evan.cheng at apple.com
Sun Sep 27 23:07:39 PDT 2009
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
>>
>>
More information about the llvm-commits
mailing list