Thx, Sun.

Since "Is_Target_SSE41()" is quite a target-specific name, we are not sure
of the original author's intension. If this part is sure of a
target-independent design, we can take this job and we should make a general
name for it.

Thanks

Gang


On Mon, Jul 11, 2011 at 8:57 PM, Sun Chan <sun.c...@gmail.com> wrote:

> Thx to Gang for pointing out the regression due to v3681 checkin. Here
> is my suggestion:
> There are multiple ways to fix the regression that will be seen in
> other target builds:
> 1. Fix based on existing infrastructure:
>    Each target must add the corresponding function Is_Target_SSE41()
> in the corresponding config_targ.h
> I agree with Gang that this fix is not desirable since it continues
> the practice that will cause target dependent build breaks easily and
> the burden lies with the receiving party, not whoever making the
> change.
> 2. Fix that could start a better change to open64 in the longer run:
>    Define a target independent Is_Target_SSE41() that will return
> "false" for all targets, and in the one that matters, return result
> appropriately.
>    One can do this in one of the following two ways I can think of:
>    a. define the function with template, and let the "target"
> specialize that accordingly. This is standard template implementation
>    b. define the generic version as weak, and let the "target"
> version that the implementor cares about to return the desired result
> as "strong". This is the implementation most libraries (like libc)
> use.
>
> Now, can I get some volunteer to go about implementing this?
>
> Sun
>
> On Fri, Jul 8, 2011 at 4:24 PM, Gang Yu <yugang...@gmail.com> wrote:
> > Hi,
> >
> >    SL target builds fail today due to the LNO check-in v3681, the build
> fail
> > comes from the patch:
> >
> > Index: config_lno.cxx
> > ===================================================================
> > --- config_lno.cxx      (revision 3643)
> > +++ config_lno.cxx      (revision 3681)
> > @@ -1224,5 +1227,12 @@
> >                         Mhd_Options.L[i].TLB_Miss_Penalty;
> >      }
> >    }
> > +
> > +  /* Value of 1 for LNO_Iter_threshold is interpreted as default in
> which
> > case
> > +     the flag is set based on target. Otherwise use user-specified
> value.
> > +   */
> > +  if(LNO_Iter_threshold == 1) {
> > +    LNO_Iter_threshold = (Is_Target_SSE41())? 8 : 0;
> > +  }
> >  }
> > Index: /home/yugang/trunk/trunk/osprey/be/cg/whirl2ops.cxx
> > ===================================================================
> > --- /home/yugang/trunk/trunk/osprey/be/cg/whirl2ops.cxx (revision 3666)
> > +++ /home/yugang/trunk/trunk/osprey/be/cg/whirl2ops.cxx (revision 3681)
> > @@ -3166,9 +3166,18 @@
> >    WN   *compare;
> >    VARIANT variant;
> >
> > +  if (opcode == OPC_V16I1V16I1SELECT) {
> > +    TN* op1 = Expand_Expr(WN_kid0(select), select, NULL);
> > +    TN* op2 = Expand_Expr(WN_kid1(select), select, NULL);
> > +    TN* op3 = Expand_Expr(WN_kid2(select), select, NULL);
> >
> > +    if (result == NULL)
> > +      result = Allocate_Result_TN (select, NULL);
> > +
> > +    Expand_Select(result, op1, op2, op3, MTYPE_V16I1, FALSE, &New_OPs);
> > //FALSE passed as dummy arg
> > +    return result;
> > +  }
> >
> > the "Is_Target_SSE41()", "OPC_V16I1V16I1SELECT" and "MTYPE_V16I1" are
> > specific to X86 targets, these patches will cause other targets fail,
> >
> > The suggest patche is:
> > Index: config_lno.cxx
> > ===================================================================
> > --- config_lno.cxx      (revision 3681)
> > +++ config_lno.cxx      (working copy)
> > @@ -1228,11 +1228,13 @@
> >      }
> >    }
> >
> > +#ifdef TARG_X8664
> >    /* Value of 1 for LNO_Iter_threshold is interpreted as default in
> which
> > case
> >       the flag is set based on target. Otherwise use user-specified
> value.
> >     */
> >    if(LNO_Iter_threshold == 1) {
> >      LNO_Iter_threshold = (Is_Target_SSE41())? 8 : 0;
> >    }
> > +#endif
> >  }
> >
> > Index: ../../be/cg/whirl2ops.cxx
> > ===================================================================
> > --- ../../be/cg/whirl2ops.cxx   (revision 3681)
> > +++ ../../be/cg/whirl2ops.cxx   (working copy)
> > @@ -3166,6 +3166,7 @@
> >    WN   *compare;
> >    VARIANT variant;
> >
> > +#ifdef TARG_X8664
> >    if (opcode == OPC_V16I1V16I1SELECT) {
> >      TN* op1 = Expand_Expr(WN_kid0(select), select, NULL);
> >      TN* op2 = Expand_Expr(WN_kid1(select), select, NULL);
> > @@ -3177,6 +3178,7 @@
> >      Expand_Select(result, op1, op2, op3, MTYPE_V16I1, FALSE, &New_OPs);
> > //FALSE passed as dummy arg
> >      return result;
> >    }
> > +#endif
> >
> >
> >
> > Would a gatekeeper help a review?
> >
> > Thanks
> >
> >
> > Gang
> >
> >
> ------------------------------------------------------------------------------
> > All of the data generated in your IT infrastructure is seriously
> valuable.
> > Why? It contains a definitive record of application performance, security
> > threats, fraudulent activity, and more. Splunk takes this data and makes
> > sense of it. IT sense. And common sense.
> > http://p.sf.net/sfu/splunk-d2d-c2
> > _______________________________________________
> > Open64-devel mailing list
> > Open64-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/open64-devel
> >
> >
>
------------------------------------------------------------------------------
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a definitive record of application performance, security 
threats, fraudulent activity, and more. Splunk takes this data and makes 
sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-d2d-c2
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel

Reply via email to