[llvm] r185993 - Un-break the buildbot by tweaking the indirection flag.

[David Blaikie]

On Wed, Jul 10, 2013 at 2:45 PM, Adrian Prantl <aprantl at apple.com> wrote:
>
> On Jul 10, 2013, at 12:58 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> On Wed, Jul 10, 2013 at 12:05 PM, Adrian Prantl <aprantl at apple.com> wrote:
>>>
>>>
>>> On Jul 10, 2013, at 12:01 PM, Eric Christopher <echristo at gmail.com> wrote:
>>>
>>>>> +        // Set the indirect flag if the type and the DIVariable's
>>>>> +        // indirect field are in disagreement: Indirectly-addressed
>>>>> +        // variables that are nonpointer types should be marked as
>>>>> +        // indirect, and VLAs should be marked as indirect eventhough
>>>>> +        // they are a pointer type.
>>>>> +        bool IsIndirect = DI->getAddress()->getType()->isPointerTy()
>>>>> +          ^ DIVar.isIndirect();
>>>>>
>>>>
>>>> This change bothers me. Why can't it be explicit what kind of location
>>>> we're expecting?
>>>
>>> The problem is the currently the semantic of the IsIndirect field in DIVariable doesn’t allow us to do that. Currently the flag indicates that "this variable is represented as a pointer”. If we change that to mean “is an indirect address”, then we could generate less ambiguous MDNodes in the frontend.
>>
>> I'm not sure I understand the distinction - but perhaps it's just a
>> matter of a poor phrasing in the comment I wrote.
>>
>> "IsIndirect" represents the case where a variable will be represented
>> by a pointer in IR but that pointer isn't the value of the variable -
>> it's a pointer to the value of the variable (as in ABIs that pass
>> non-trivial C++ object parameters as pointers).
>>
>> Could you describe in more detail the distinction you're making? I'm
>> not sure I'm following.
>
> Even better! That sounds just like my definition of indirect addressing above.
> In that case I will change that line to
>
>> bool IsIndirect = DIVar.isIndirect();
>
> which should align with everyones intuitive expectation anyway, and perform whatever changes are necessary to make that actually work as expected.

OK, still confused by what you thought this was about, but that's not
important to discuss.

So we're on the same page: isIndirect implies that the IR type is a
pointer. That was my intention, at least - is that clear/agreed?

> I will also extend the comment of DIVariable::IsIndirect to make this clearer.
>
>
> thanks,
> adrian