On the 0x216 day of Apache Harmony Konstantin Anisimov wrote:
> Hi all,
>
> I suggest new patch from the series Igor introdusced.
> 1. To move direct predicated calls in separete node. It allows to have under
> predicate short branch instruction instead of call and thus
> reduce possible misprediction penalty.
> 2. I have implemented new node merging algorithm. It is more effective than
> previouc one and besides purging empty nodes.
>
> All changes made in Code layouting and I suggest integrate them in one
> patch.
Konstantin,
I took a quick look at the HARMONY-2061 you are introducing. Changes
look generally good to me, but I have some suggestions (some minor and
some requiring to update the patch).
Here they are for the easier reply. I'll duplicate them in JIRA for
the easier tracking.
Changes do *not* affect the IA-32 build. Which is great! (I verified
this)
I am slightly unhappy with changes like:
------------------------------------------------------------
+++ include/IpfCodeLayouter.h (working copy)
@@ -17,7 +17,7 @@
/**
* @author Intel, Konstantin M. Anisimov, Igor V. Chebykin
- * @version $Revision$
+ * @version $Revision: 1.1.1.6 $
*
*/
------------------------------------------------------------
is it easy to overcome them? maybe, remove the $Revision$ keyword at
all? too CVS-ish, not for today
Why do you use 'map' instead of more commonly used StlMap
(MemoryManager based)? For the sake of safety to memory leakage we
use memory managers (and allocate them on stack). I understand that
your map is allocated on stack, and, hence induces no memory leakage,
but it is not like the common style we use. Someone can allocate the
map in heap by mistake. Could you, please, make it an StlMap?
I see some bugfixes. Cool :)
here:
------------------------------------------------------------
@@ -436,7 +538,8 @@
// Add branch to through edge target
Opnd *p0 = cfg.getOpndManager()->getP0();
NodeRef *targetNode = cfg.getOpndManager()->newNodeRef(target);
- node->addInst(new(mm) Inst(INST_BR, CMPLT_BTYPE_COND, p0, targetNode));
+// node->addInst(new(mm) Inst(mm, INST_BR, CMPLT_BTYPE_COND, CMPLT_WH_SPTK,
p0, targetNode));
+ node->addInst(new(mm) Inst(mm, INST_BR, CMPLT_WH_SPTK, CMPLT_PH_FEW, p0,
targetNode));
------------------------------------------------------------
does it make sense to leave this line commented-out? Please, remove it
completely.
Minor suggestion: please, provide patches for the "working_vm"
directory, or one level above, otherwise it needs an extra guess where
to apply it (this time it is vm/jitrino/src/codegenerator/ipf)
RegOpnd constructor signature changed, but no changes followed in the
implementation
(IpfOpnd.cpp). You probably forgot to include the file into the diff. Please,
update.
Do I get it right that the new CFG verifier is going to be put in the
IpfCfgVerifier.{cpp|h}? Is it going to be worked out soon? Who is
working on it?
> "Igor Chebykin" <[EMAIL PROTECTED]> wrote in message
> news:[EMAIL PROTECTED]
> Hello all,
>
> I suggest a short series of patches for drlvm ipf code generator.
> We have some improvements for jitrino/ipf
> and would like to commit its to harmony.
>
> All patches will change only vm/jitrino/src/codegenerator/ipf/* files,
> therefore ia32 remains OK.
>
> The first patch is about 67k size and contains following files:
> IpfCfg.h, IpfCfg.cpp
> methods added in Edge and Node classes
> IpfCodeLayouter.h, IpfCodeLayouter.cpp
> new BotomUp algorithm implementation
> IpfEmitter.h, IpfEmitter.cpp
> minor changes in logging, Emitter::registerDirectCall() and
> debugging support
> IpfIrPrinter.h, IpfIrPrinter.cpp
> added method to print Node chains
> IpfType.h
> types to support Node chains added
> IpfCfgVerifier.cpp
> method cfg.getArgs() deprecated
> IpfInst.cpp
> methods to identify inst kind added (isBr, isCall
)
> IpfRegisterAllocator.cpp
> minor changes in logging
>
> Thanks,
> Igor.
>
>
> --
> Igor Chebykin, Intel Middleware Products Division
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>
>
>
>
>
--
Egor Pasko
