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

[Adrian Prantl]

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.

I will also extend the comment of DIVariable::IsIndirect to make this clearer.


thanks,
adrian