Your new patch looks good to me. Please go ahead.
Thanks,
Min
On Wed, Oct 26, 2011 at 9:40 PM, Hui Shi <kalin....@gmail.com> wrote:
> Hi Min,
>
> Thanks for your review comments. Comments below.
> I made some minor updated and attach a new patch.
> Please help check.
>
> Regards
> Shi Hui
>
> On Thu, Oct 27, 2011 at 5:25 AM, Min Zhao <mzhao...@gmail.com> wrote:
>>
>> Hi Hui,
>>
>> Your change looks good to me in general. I agree with your approach to
>> have one NystromAliasAnalyzer in IPA phase, most importantly having
>> one alias tag map, which makes alias query for WNs in different PUs
>> easy. Just some minor comments:
>>
>> 1) You have some places to access the data member (e.g.,
>> _IPAAliasTageMap) directly. Can you please use the get/set function to
>> access them?
>
> _IPAAliasTageMap is only used in its belonging class AliasAnalyzer.
> And similarly _aliasTagMap is also directly used in AliasAnalyzer, so can we
> keep the alignment of coding stype in AliasAnalyzer? Leaving all member
> directly used in its own class.
>
>>
>> 2) why we need to set Write_ALIAS_CGNODE_Map to be true in the new
>> NystromAliasAnalyzer constructor? Do we need to dump the WN to
>> CGNodeID map?
>
> You're right.Write_ALIAS_CGNODE_Map is already set true in
> IPA_NystromAliasAnalyzer constructor.
> So there is no need to update Write_ALIAS_CGNODE_Map again in
> NystromAliasAnalyzer constructor.
> I have fix this in new patch.
>
>>
>> 3) your two functions IPANodeCG()(one is get function, the other is
>> set function) are called same name, which is a little bit confusing.
>
> This also keep align with the exsting code.
> static ConstraintGraph *globalCG() { return globalConstraintGraph;
> }
> static void globalCG(ConstraintGraph *cg) { globalConstraintGraph = cg; }
>
> static CG_ST_IDX adjustCGstIdx(IPA_NODE *ipaNode, CG_ST_IDX cg_st_idx);
>
> static ConstraintGraph *IPANodeCG() { return
> currentIPANodeConstraintGraph; }
>
> static void IPANodeCG(ConstraintGraph* cg) {
> currentIPANodeConstraintGraph = cg; }
>
>>
>> 4) should the map _aliasTagMap deleted before the _memPool is deleted?
>
>
> Yes, this seems unreasonable code that mem_pool is freed before WN_Map is
> removed.
> This doesn't cause problem, because in IPA_WN_MAP_Delete, it only free
> memory when pool is Malloc_Mem_Pool.
> Otherwise it will free the already freed memory.
> Fix the order of WN_MAP delete.
>
>>
>> Thanks,
>>
>> Min
>>
>> On Thu, Jul 28, 2011 at 12:43 AM, Hui Shi <kalin....@gmail.com> wrote:
>> > Hi, All
>> >
>> > Can gatekeepers help review this patch?
>> >
>> > When apply option -OPT:alias=field_sensitive -IPA:preopt=on, compiler
>> > will
>> > assert in NystromAliasAnalyzer.
>> > NystromAliasAnalyzer assumes it is in be phase and try to get the IPA
>> > constraint graph summary from IR file.
>> >
>> > To enable nystrom alias analyzer works with pre_optimizer in IPA phase
>> > need
>> > create
>> > NystromAliasAnalyze from global contraint graph and IPA Node's local
>> > constraint graph.
>> >
>> > This patch is:
>> > 1. Create NystromAliasAnalyzer for PU in IPA phase.
>> > 2. Provide alias query interface for WN nodes or alias tags in different
>> > PU.
>> >
>> > Key decision is only use one NystromAliasAnalyzer instance in IPA phase.
>> > 1. Multiple PU's alias tag in one alias tag space.1
>> > 1. Ease alias query, if same alias tag for WN node in different, they
>> > are
>> > aliased.
>> > 2. Ease alias query cache. NystromAliasAnalyzer cache alias query
>> > result,
>> > key is
>> > alias tag pair. If create one NystromAliasAnalyzer for each PU.
>> > It's
>> > complex to
>> > cache qurey of alias tags from different PU.
>> > 3. Save space for alias tag's points_to set .
>> > In different PU, there is alias tags that their points_to set is
>> > euqivalent.
>> > For example, actual and formal paramters' points to set is same.
>> > Only use one copy points to set can save memory.
>> > 2. AliasAnalyzer::aliasAnalyzer() is a static method.
>> > Current code assumes only one alias analyzer instance.
>> >
>> > Implementation:
>> > Add globals:
>> > 1. Alias_Analyzer_in_IPA
>> > set true in IPA_Preoptimize when Alias_Nystrom_Analyzer is on.
>> > 2. Current_IPANode_File_PU_Idx
>> > current IPA node's file PU index in IPA_Preoptimize.
>> >
>> > In aliasAnalyzer
>> > Add following fields
>> > 1. IPAWNAliasTagMap _IPAAliasTageMap; map WN node to its alias tag.
>> > Key is UINT64, (IPA Node file_pu_idx << 32) | wn_map_id
>> > 2. bool _ipaMode; indicate if NystromAliasAnalyzer is created in IPA
>> > phase
>> > 3. UINT32 _curFilePUIdx; // current processing IPA node's file and PU
>> > index
>> >
>> > Update AliasAnalyzer::Create_Alias_Analyzer(ALIAS_CONTEXT &ac, WN *tree)
>> > if Alias_Analyzer_in_IPA, set _ipaMode true and set _curFilePUIdx.
>> >
>> > Update interface
>> > void setAliasTag(WN *wn, AliasTag tag)
>> > AliasTag getAliasTag(const WN *wn) const
>> > to
>> > void setAliasTag(WN *wn, AliasTag tag, UINT32 filePUIdx=UINT32_MAX)
>> > AliasTag getAliasTag(const WN *wn, UINT32 filePUIdx=UINT32_MAX) const
>> > In _ipaMode, compose filePUIdx with wn_map_id as key, find or set alias
>> > tag
>> > map.
>> >
>> > In NystromAliasAnalyzer
>> > Add new constructor NystromAliasAnalyzer::NystromAliasAnalyzer
>> > only set _ipaMode is true.
>> > Not build constraint graph or create alias tag.
>> >
>> > Update NystromAliasAnalyzer::genAliasTag(ST *st, INT64 offset, INT64
>> > size,
>> > bool direct)
>> > When getting St_Info for ST.
>> > 1. If ipaMode and St is global, search St_Info in IPA global constraint
>> > graph.
>> > 2. If ipaMode and St is local st, search St_Info in local constriant
>> > graph,
>> > key is
>> > File PU idx + ST idx
>> > 3. Not in ipaMode, search in local constraint graph, key is ST idx.
>> >
>> > In ConstraintGraph
>> > Add static ConstraintGraph *currentIPANodeConstraintGraph;
>> > Pass current processing IPA node's constraint graph from
>> > IPA_Preoptimize.
>> > Used in AliasAnalyzer::Create_Alias_Analyzer, if in IPA phase, set
>> > current
>> > constraint graph with currentIPANodeConstraintGraph.
>> >
>> > Add bool _uniqueMapped; set true when current constraint graph is
>> > processed
>> > by
>> > IPA_NystromAliasAnalyzer::mapWNToUniqCallSiteCGNodeId.
>> >
>> > IPA_NystromAliasAnalyzer::mapWNToUniqCallSiteCGNodeId currently only
>> > called
>> > before
>> > write IPA nystrom alias info into IR file.
>> > It is also needed called before create alias tag in IPA_preoptimize.
>> > Use this flag to avoid map twice.
>> >
>> > Regards
>> > Shi Hui
>> >
>> >
>> >
>> >
>> >
>> > ------------------------------------------------------------------------------
>> > Got Input? Slashdot Needs You.
>> > Take our quick survey online. Come on, we don't ask for help often.
>> > Plus, you'll get a chance to win $100 to spend on ThinkGeek.
>> > http://p.sf.net/sfu/slashdot-survey
>> > _______________________________________________
>> > Open64-devel mailing list
>> > Open64-devel@lists.sourceforge.net
>> > https://lists.sourceforge.net/lists/listinfo/open64-devel
>> >
>> >
>
>
------------------------------------------------------------------------------
The demand for IT networking professionals continues to grow, and the
demand for specialized networking skills is growing even more rapidly.
Take a complimentary Learning@Cisco Self-Assessment and learn
about Cisco certifications, training, and career opportunities.
http://p.sf.net/sfu/cisco-dev2dev
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
