[LLVMdev] ValueTracking's GetUnderlyingObject vs. ScheduleDAGInstrs' getUnderlyingObject

[Matthew Curtis]

On 10/15/2012 11:23 AM, Duncan Sands wrote:
> Hi Matthew,
>
>> I'm investigating a problem with "Machine Instruction Scheduling" 
>> that is rooted
>> in incorrect alias information.
>>
>> Things go wrong in the "Merge disjoint stack slots"[1] pass when a store
>> instruction fails to get updated after its stack slot is merged.  As 
>> a result,
>> when "Machine Instruction Scheduling" runs, it incorrectly re-orders 
>> the store
>> and a subsequent load, thinking that they do not refer to the same 
>> underlying
>> object when they actually do.
>>
>> The logic in "Merge disjoint stack slots" seems ok, except that it 
>> relies on
>> llvm::GetUnderlyingObject[2] to determine if an instruction needs to 
>> be updated.
>
> it sounds to me like this logic is broken.  For example, 
> GetUnderlyingObject
> has a recursion limit that will bail out if it has to look through too 
> many
> instructions to find the underlying object.  Thus you can't ever rely on
> GetUnderlyingObject actually finding the real underlying object. But it
> sounds like "Merge disjoint stack slots" *is* relying on this (I don't 
> know the
> code, it's just the impression I get from your description).
>
> Ciao, Duncan.
>

Yes, After speaking with Arnold offline as well, I have come to realize 
that "Merge disjoint stack slots" is not correctly using 
GetUnderlyingObject. While we could still improve GetUnderlyingObject so 
that the pass works correctly in more cases, it still needs to 
conservatively handle the possibility that GetUnderlyingObject may not 
return the real underlying object.

I will be filing a bug shortly.

Thanks,
Matthew Curtis.


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation