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

Reid Kleckner rnk at mit.edu
Sun Sep 27 23:17:41 CDT 2009


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 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.

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