Hi Sun,
Thank you for the comments.
For #1, I'll change the code according to your suggestion.
For #2, I still think the loop is necessary. Considering the example:
try {
try {
...
stmt A; // stmt A is the last statement of BB1
}
catch(...) {
}
}
catch(...) {
}
try {
try {
stmt B; // stmt B is the first statement of BB2
...
}
catch(...) {
}
}
catch(...) {
}
BB1 is the immediate predecessor of BB2. Since there is no goto at the end
of BB1, the last statement of BB1 and first statement of BB2 should be
'adjacent'. So we need to trim the nested regions to check if stmt A and
stmt B is 'adjacent'. What's your opinion?
For #3, I will enclose all code in WN_CFG::Verify() into #ifdef Is_True_On.
The code invoked by WN_CFG::Verify() only will not be enclodes by #ifdef
Is_True_On to make the code more clean.
2011/2/3 Sun Chan <sun.c...@gmail.com>
> 1. You are basically using trace and is_true to be your comment and
> for people to understand your logic. It is good to use lots of asserts
> and traces, but don't forget that it obscures the program flow. so it
> is a fine line to decide what is the right course of action. E..g the
> first portion, I would simply check that res and opnd are what you
> expect first, then check that if they are non-null, return. My
> assertion will be after the check. This way, the code is very obvious
> and assertion + trace is still there.
> Also, the 3 portions of code are almost the same, have you considered
> a function or template with function object?
>
> 2. Is_statement_adjacent implementation is funny, although the
> do-whiles should traverse only once, the code could potentially loop.
> Is there a better way to implement this? The implementation assumes a
> lot about inherent property of the cfg
>
> 3. I assume the verifiiers will be called inside Is_True (is this
> clarified in the declaration to warn people?). The actual code (e.g.
> Finalize() is not enclosed in Is_True.
>
> Otherwise, the code looks good.
> Sun
>
>
> On Thu, Feb 3, 2011 at 12:14 AM, Jian-Xin Lai <laij...@gmail.com> wrote:
> > Hi Sun,
> >
> > Could you please review the code for WHIRL CFG verfier and 1 bug fix for
> > WHIRL SSA in mainopt emitter? Thank you very much.
> > The WHIRL CFG verifier shares the same traversal code with the WHIRL CFG
> > builder to verify the CFG.
> > The bug fix for WHIRL SSA in MainOPT emitter is to remove duplicated
> > phi/chi/mu node of the same symbol attached on the same whirl node. This
> > case happens when two WOPT stmbol entries are mapping to one symbol in
> WHIRL
> > SSA.
> >
> > The patch is attached. Thank you very much.
> >
> > --
> > Regards,
> > Lai Jian-Xin
> >
>
--
Regards,
Lai Jian-Xin
------------------------------------------------------------------------------
The ultimate all-in-one performance toolkit: Intel(R) Parallel Studio XE:
Pinpoint memory and threading errors before they happen.
Find and fix more than 250 security defects in the development cycle.
Locate bottlenecks in serial and parallel code that limit performance.
http://p.sf.net/sfu/intel-dev2devfeb
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
