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
> >
> >
>
nystrom_ipa_preopt_review_1.patch
Description: Binary data
------------------------------------------------------------------------------ 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
