I think keeping Is_Target_SSE41 undefined on other platforms will help
find any potential problems on other platforms caused by x86 specific
changes.
For example, in this case, changing Is_Target_SSE41 to return false on
other platforms will change the original program's semantic. If we
keep Is_Target_SSE41 undefined, we are able to find the problem from
the compilation error and check the initial and expected value of
LNO_Iter_threshold to make sure if this change will impact the target.
2011/7/12 Sun Chan <sun.c...@gmail.com>:
> 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
>
--
Regards,
Lai Jian-Xin
------------------------------------------------------------------------------
AppSumo Presents a FREE Video for the SourceForge Community by Eric
Ries, the creator of the Lean Startup Methodology on "Lean Startup
Secrets Revealed." This video shows you how to validate your ideas,
optimize your ideas and identify your business strategy.
http://p.sf.net/sfu/appsumosfdev2dev
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
