[llvm-commits] [PATCH] Memory Dependence Analysis: differenciate "unknown" from "no dependency in current function"

[Eli Friedman]

On Wed, Oct 12, 2011 at 2:20 PM, Guo, Xiaoyi <Xiaoyi.Guo at amd.com> wrote:
> Hi Eli,
>
> Thanks a lot for the feedback. Here's another try according to your suggestion. Please help review it again.

@@ -1364,7 +1366,7 @@
       continue;
     }

-    assert(DepInfo.isDef() && "Expecting def here");
+    // DepInfo.isDef() here

     Instruction *DepInst = DepInfo.getInst();

What is the point of this change?

+      /// Other - This marker indicates that the query has no dependency in
+      /// the specified block.  More detailed state info is encoded in the
+      /// upper part of the pair (i.e. the Instruction*)

I would say "no known dependency" instead of "no dependency".

Otherwise, this patch looks good.

-Eli

> Xiaoyi
>
> -----Original Message-----
> From: Eli Friedman [mailto:eli.friedman at gmail.com]
> Sent: Tuesday, October 11, 2011 5:27 PM
> To: Guo, Xiaoyi
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [PATCH] Memory Dependence Analysis: differenciate "unknown" from "no dependency in current function"
>
> On Tue, Oct 11, 2011 at 4:57 PM, Guo, Xiaoyi <Xiaoyi.Guo at amd.com> wrote:
>> If the memory dependence analysis did not find any dependence for a pointer in the current function, it currently returns "Unknown" as the result. However, in the optimization that I'm doing, I need to be able to tell this case apart from a general "unknown" case. i.e., I need to know that the pointer does not have any dependencies in the current function. The patch attached is to achieve this.
>>
>> Please help to review and commit if acceptable.
>
> The interface to MemDepResult::getNonFuncLocal makes no sense: as far as I can tell, the Instruction* which is passed in is useless except for the fact that it isn't null.
>
> MemDepResult's internal representation is starting to get a bit crazy... with your changes, there are 6 states: Invalid, Def, Clobber, NonLocal, NonLocalFunc, and Unknown.  Def and Clobber have an associated instruction, the other states do not.  Granted, I sort of started the craziness by the way Unknown is represented, but this is starting to become a complete mess; maybe we can restore a bit of sanity by splitting things up so that the bottom part of the pair is "Def", "Clobber", or "Other", and encode the other states into upper part of "Other", or something like that?
>
> -Eli
>
>