harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Egor Pasko <egor.pa...@gmail.com>
Subject Re: [drlvm] [ipf] I suggest a series of patches for ipf code generator
Date Tue, 07 Nov 2006 12:38:23 GMT
On the 0x21A day of Apache Harmony Konstantin Anisimov wrote:
> Egor,
> 
> thank you for the suggestions. I have taken them into account prepareing 
> next patch version.
> Please find it in Jira HARMONY-2061 request.

Konstantin, thank you for updating:
* map -> StlMap
* removed "Revision" substrings

But several issues are not resolved as suggested. Let's discuss those
and converge fast. Here they are one by one. Please, comment:

* RegOpnd::RegOpnd is still not updated according to the changes in
  the .h file. Does it compile on IPF?

* IpfCodeLayouter.cpp:436: commented code is still there. Is there any
  reason for that?

-    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));

* the relative path is still
  'working_vm/vm/jitrino/src/codegenerator/ipf', but 'working_vm' would
  be better. (this is a minor issue, just for future)

> "Egor Pasko" <egor.pasko@gmail.com> wrote in message 
> news:u4ptb7obt.fsf@gmail.com...
> > 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" <iche.harmony@googlemail.com> wrote in message
> >> news:e1c5d0b40610200524x5d1fa18di3a348721f2dcba63@mail.gmail.com...
> >> 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: harmony-dev-unsubscribe@incubator.apache.org
> >> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> >>
> >>
> >>
> >>
> >>
> >
> > -- 
> > Egor Pasko
> >
> > 
> 
> 
> 
> 

-- 
Egor Pasko


Mime
View raw message