WHy do you like the first solution? It added yet another #ifdef which is
1. going against our wish to do fewer #ifdef's
2. the whole point of having things like Is_target_xxx() is to do away
with #ifdef's
Sun
On Wed, Jul 20, 2011 at 3:47 AM, Jian-Xin Lai <laij...@gmail.com> wrote:
> Actually, I prefer the first patch. Let's wait for Sun's comments on this
> issue.
>
> 2011/7/20 Gang Yu <yugang...@gmail.com>
>>
>> Jianxin, Sun;
>>
>> I get another solution for this Is_Target_SSE41() problem, I suggest a
>> patch like this:
>>
>> --- osprey/common/com/config_lno.cxx (revision 3697)
>> +++ osprey/common/com/config_lno.cxx (working copy)
>> @@ -1232,7 +1232,7 @@
>> 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;
>> + LNO_Iter_threshold = (Is_Aggressive_SIMD_Target())? 8 : 0;
>> }
>> }
>>
>> --- osprey/common/com/SL/config_targ.h (revision 3697)
>> +++ osprey/common/com/SL/config_targ.h (working copy)
>> @@ -161,6 +161,7 @@
>> #define Is_Target_ISA_M2Plus() (Target_ISA >= TARGET_ISA_M2)
>> #define Is_Target_ISA_M3Plus() (Target_ISA >= TARGET_ISA_M3)
>> #define Is_Target_ISA_M4Plus() (Target_ISA >= TARGET_ISA_M4)
>> +#define Is_Aggressive_SIMD_Target() (FALSE)
>>
>> --- osprey/common/com/x8664/config_targ.h (revision 3697)
>> +++ osprey/common/com/x8664/config_targ.h (working copy)
>> @@ -194,8 +194,8 @@
>> #define Target_x87_precision() (Target_x87_Precision+0)
>> #define Is_Target_Barcelona() (Target == TARGET_barcelona)
>> #define Is_Target_Orochi() (Target == TARGET_orochi)
>> +#define Is_Aggressive_SIMD_Target() (Target_SSE41 == TRUE)
>>
>> with this patch, we do not introduce the #ifdef in the target independent
>> sections and also keep the capabilitity to share the code. The patch has
>> been tested on x86 and SL targets.
>>
>> The attached patch also solves other build broken problems for non-x86
>> targets.
>>
>> Please help a review.
>>
>> Gang
>>
>>
>> On Mon, Jul 18, 2011 at 4:59 PM, Jian-Xin Lai <laij...@gmail.com> wrote:
>>>
>>> Hi Sun,
>>>
>>> I think Gang's first patch is good enough to fix the compilation error on
>>> both IA-64 and SL. We can let him check in the patch at first and continue
>>> discussing about how to handle the Is_Target_SSE41 (and similar macros).
>>> What's your opinion?
>>>
>>> 2011/7/15 Jian-Xin Lai <laij...@gmail.com>
>>>>
>>>> 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
>>>
>>>
>>>
>>> --
>>> Regards,
>>> Lai Jian-Xin
>>
>
>
>
> --
> Regards,
> Lai Jian-Xin
>
------------------------------------------------------------------------------
10 Tips for Better Web Security
Learn 10 ways to better secure your business today. Topics covered include:
Web security, SSL, hacker attacks & Denial of Service (DoS), private keys,
security Microsoft Exchange, secure Instant Messaging, and much more.
http://www.accelacomm.com/jaw/sfnl/114/51426210/
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
