Hi Chris:
1. Variadic macros appear to be a GCC extension. Our base language for backend
support is C99, so more recent additions to the language cannot be used. This
is to maintain backend portability.
2. I like the conversion of registerParamPreference into a static function. It
could be marked "inline" to more closely mimic its definition as a macro.
However, GCC is smart enough to figure out whether it can be benefically
inlined, even without the hint.
3. I was hoping to see a solution that roughly boiled down to:
score1 = scoreMatch(candidateFn1, info.actuals, actualFormals, scope, ...);
score2 = scoreMatch(candidateFn2, info.actuals, actualFormals, scope, ...);
if (score1 < score2) worse = true;
if (score1 == score2) equal = true;
// Same loop logic
One could argue that there is some performance gain due to early exit when
the first candidate is found to be worse. But I doubt it. Integer or boolean
arithmetic is at least 10 times faster than chained decisions. So replacing
all those "if"s with a simple algebraic expression to compute a score is likely
to be a net win for performance.
In any case, it is axiomatic that "repeated code" and "early optimization"
are bug farms, so finding a solution that avoids both will be a boon.
Another possible implementation is to carry along the set of candidates, and
apply successively more restrictive filters. If at any point the set of
candidates contains exactly one member, a match has been found. If it contains
zero, then the result is ambiguity up to the application of the current filter.
(For error reporting, one would maintain a copy of the previous candidate set,
so the conflicting candidates could be listed.)
4. That said, I think your factoring still represents an improvement. So if
you don't feel inclined to rewrite it, we can proceed. Let me know if you
stand behind this patch, so I can apply it and give it a more detailed review
it "in situ". I have been looking only at the patch file, and might appreciate
it more by viewing the old and new side by side.
5. In any case, it would be helpful if you added a brief header comment for
each new class you have introduced, describing what it represents and how it is
used.
Please respond,
Tom H.
________________________________
From: Tom Hildebrandt [[email protected]]
Sent: Tuesday, November 19, 2013 10:20 PM
To: Brad Chamberlain; Chris Wailes; [email protected]
Subject: Re: [Chapel-developers] Request for Review: Refactoring Disambiguation
By Match
All:
I have considered rewriting this function, so I may have some useful insights.
I will review it and post my comments tomorrow.
Tom H.
________________________________
From: Brad Chamberlain [[email protected]]
Sent: Tuesday, November 19, 2013 9:12 PM
To: Chris Wailes; [email protected]
Subject: Re: [Chapel-developers] Request for Review: Refactoring Disambiguation
By Match
I've been working in this part of the code a lot lately (relatively to the rest
of the group), and so would be a logical reviewer, and am definitely interested
in reviewing it, but am not certain that I'll have time to before the end of
Thanksgiving week (and then will have some digging out from backlog to do). If
someone else from the core team wants to review it in the meantime, I can
review it post-commit after I get back; or if Chris can wait, I can do it then.
-Brad
________________________________
From: Chris Wailes [[email protected]]
Sent: Tuesday, November 19, 2013 3:50 PM
To: [email protected]
Subject: [Chapel-developers] Request for Review: Refactoring Disambiguation By
Match
This patch is a refactoring of the disambiguation_by_match function. It cleans
up the function interface, splits the functionality into several
easier-to-understand functions, and renames variables (and rearranges some
Boolean conditionals to have the naming make sense) to make their function
clearer. In addition, information from comparisons is recorded and reused to
skip over functions that can't be the best match. Lastly, the tracing enabled
by --explain-call has been improved.
- Chris
------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing
conversations that shape the rapidly evolving mobile landscape. Sign up now.
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
_______________________________________________
Chapel-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/chapel-developers
