[llvm] r176408 - recommit r172363 & r171325 (reverted in r172756)
Arnold Schwaighofer
aschwaighofer at apple.com
Sun Mar 3 15:55:15 PST 2013
On Mar 3, 2013, at 5:38 PM, Hal Finkel <hfinkel at anl.gov> wrote:
>>> Let me give an example:
>>>
>>> %p1 = malloc(42);
>>> %p2 = malloc(42);
>>> %p3 = phi(%p1, %p2);
>>> %size = @llvm.objectsize(%p3);
>>>
>>> The patch I commited enables instcombine to replace %size with the
>>> constant 42.
>>
>> Yes, and that is the problem. I claim that it is questionable whether
>> the semantics of "llvm.objectsize" does allow you to do so.
>>
>> "The llvm.objectsize intrinsic is lowered to a constant representing
>> the size of THE object concerned"
>> - http://llvm.org/docs/LangRef.html#llvm-objectsize-intrinsic
>>
>> There is no mentioning of more than one object. I believe you could
>> interpret this as a contract that llvm.objectsize will only return a
>> valid size value for one identified underlying object.
>>
>> "An object in this context means AN allocation of A specific class,
>> structure, array, or other object."
>> - http://llvm.org/docs/LangRef.html#llvm-objectsize-intrinsic
>>
>> Again, no mentioning of a set of objects. To me this reads as a clear
>> specification of what the object is. ONE and not a set.
>>
>> If you think that the semantics of "llvm.objectsize" should be
>> different in that it allows for a set of underlying objects, again,
>> I think we have to change the LangRef. We might be breaking out of
>> tree code that interpreted the semantics as I did, though.
>>
>> What I am concerned with is external code that took my interpretation
>> of what llvm.objectsize means:
>>
>> Imagine a code generator taking my view and generating the following
>> code:
>>
>> %p1 = malloc(42);
>> %p2 = malloc(42);
>> %p3 = phi(%p1, %p2);
>> %size = @llvm.objectsize(%p3, true);
>>
>> if (%size != 0) {
>> free(%p1)
>> } else {
>> free(%p1)
>> free(%p2)
>> }
>
> I'll comment from the sidelines that, while I can see this is a theoretical possibility, we should not use it as a justification for inhibiting a useful optimization. Nevertheless, this seems to be a solvable problem. For example, we could add a third parameter to the objectsize intrinsic indicating whether ambiguous object sizes are allowed, and we could auto-upgrade the two argument form to the three argument form such that the new argument defaults to false.
Hal, I am not arguing for preventing useful optimizations. I am arguing for clarifying semantics. I will go away on the "llvm.objectsize.i32/i64" issue if the community agrees that my interpretation is wrong/not possible. Otherwise, I remain concerned what we map it to.
Yes, what you are proposing is one way of dealing with a possible change in semantics of llvm.objectsize. But as it stands, that is not what the patch we are arguing over does.
More information about the llvm-commits
mailing list