just return false if it does not apply to you
Sun
On Tue, Jul 12, 2011 at 9:25 AM, Gang Yu <yugang...@gmail.com> wrote:
> 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
