[lldb-dev] Question about IRForTarget

[David Blaikie]

On Mon, Mar 16, 2015 at 10:45 AM, Zachary Turner <zturner at google.com> wrote:

> So assuming the correct type is passed today, is that semantically
> equivalent to passing nullptr today?
>

Right - for now the new parameter is not used at all, except in the
assertion. So there's no way to get a change in behavior - either nullptr
is passed and nothing happens, or a type is passed and if the type matches,
nothing happens - if you get the type wrong, the assertion will fail.

This is a rather large migration effort, so I'm trying to take as small
steps as possible to ensure any issues are flushed out at each step before
changing the semantics out from underneath users.


> Because if not, then I would have expected a test to fail somewhere.  Not
> because an assertion fired, but because something somewhere had the wrong
> idea about a type.
>
> On Mon, Mar 16, 2015 at 10:43 AM David Blaikie <blaikie at google.com> wrote:
>
>> On Mon, Mar 16, 2015 at 10:39 AM, Zachary Turner <zturner at google.com>
>> wrote:
>>
>>> Hi Sean,
>>>
>>> A change went in over the weekend to change the
>>> way GetElementPtrInst::Create works.  Now, for pointer types, you need to
>>> manually specify the type of the pointee, as pointerType->getType() won't
>>> work anymore.  This broke the build, so a workaround was employed to simply
>>> pass nullptr for this argument.  But this is likely to be incorrect, and
>>> instead we need the actual llvm::Type for the pointee.  I've looked over
>>> the code, and I *think* that on line 2209 we can replace the nullptr with
>>> old_constant->getType(), and on line 2396 we can place the nullptr with
>>> value->getType().
>>>
>>> What are your thoughts here?
>>>
>>> Secondly, the change to nullptr does not seem to have caused any test
>>> failures.
>>>
>>
>> This is expected - for now, nullptr is an acceptable value. The assertion
>> I have in GetElementPtrInst's ctor is:
>>
>> assert(!PointeeType || PointeeType == getSourceElementType());
>>
>> Eventually I'll need to remove the nullptr case once I believe I've
>> flushed out all the callers - this will verify that the callers are passing
>> the type that matches the pointee type of the pointer operand. (at this
>> point the current lldb code will assert-fail, if it is tested) You can try
>> removing the first part of that assert locally to see if the LLDB code path
>> is tested currently.
>>
>> Once similar changes have been applied to all pointer-element-type-aware
>> uses of pointers in LLVM IR, I'll start trying to remove the ability to
>> query a pointer type for its element type.
>>
>>
>>> Is there any way we can add some tests for this?  If this is not
>>> something that is testable through the public interface, I recently added
>>> gtest unit tests to the build.  It's not enabled in the Xcode build (but
>>> will be soon), but if you tell me how to set up a test, call the right
>>> function, and what outputs to expect, I can write it.
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20150316/9aca5bdf/attachment.html>