[llvm] r179111 - Revert r176408 and r176408 to address PR15540.

Shuxin Yang shuxin.llvm at gmail.com
Mon Apr 15 11:15:28 PDT 2013


Hi, Nuno:

     I re-read the code this morning, and try to recall the bug I fixed,
I'd take back what I said as to r176407 is able to catch more cases.

  This is original relevant code sequence:

sequence 1:
   -----------------------
    O = getUnderlyingObject()

    if (O is pointing to object)
       give up;

    size = getObjectSize(O);
   ------------------------

  While this is your change in 176407:
sequence 2:
   =================
    O = getUnderlyingObject()

    size = getUnderlyingObjectSize();
    ===================

  Without 176407+1, sequence 1 and 2  are identical in functionality 
because:
    - if the O is actually points to a object,
       getObjectSize() , and getUnderlyingObjectSize() are no different.

    - if the O is not pointing to an object,
       with 176407 only, getUnderlyingObjectSize() is supposed to return 
"invalid".

   The sequence is  win in compile time because, more often than not,  O 
is not
pointing to an object, sequence 1 exit earlier, obviating the need to 
calling
getUnderyingObjectSize().

   While the getUnderyingObjectSize() is relatively inexpensive for 
now,  it could be expensive
in the future, as your 176408 already prove it.

Shuxin

>> If you really want r176407 back,  I don't have strong objection. If 
>> you are seeking a better way, I think it would better hold it for a 
>> while:-).
>
> Ok, thanks!
> Nadav: if you're still following this thread :),  can you recommit 
> r176407 for me, please?
> I believe it's a slightly better solution and enables future 
> improvements.
>
> Thanks,
> Nuno




More information about the llvm-commits mailing list