Mike also suggested another way that YongChong adopted in one recent change.
I also mentioned using templates and partial specialization that you
did not include.
There are multiple ways. #ifdef is known to cause readability and
maintenability problem. It is fastest to implementors, but causes most
problem. No, not unless there is no other alternative.
Sun
On 9/7/11, Gang <yugang...@gmail.com> wrote:
> Sun:
>
> It's again the "#ifdef" problem:( AFAIK, there are the following
> suggestions , practices regarding on this CG porting issue:
>
> a.) common interface, general "assert" implementation, target specific
> real work done.
>
> typical snippet:
> cg/cgtarget.h
> extern void CGTARG_Perform_THR_Code_Generation(OP *load_op, OP
> *check_load, THR_TYPE type);
>
> cg/SL/cgtarget.cxx
> void CGTARG_Perform_THR_Code_Generation (OP *load_op, OP *chk_load,
> THR_TYPE type)
> {
> FmtAssert(FALSE,("NOT YET IMPLEMENTED"));
> }
>
> cg/ia64/cgtarg.cxx:
> void CGTARG_Perform_THR_Code_Generation (OP *load_op, OP *chk_load,
> THR_TYPE type)
> {
> if (type & THR_DATA_SPECULATION_NO_RB) {
> FmtAssert(OP_load(load_op), ("CGTARG_Perform_THR_Code_Generation :
> not a load OP"));
> INT enum_pos = -1;
> ...
>
> with this approach, there are no #ifdef introduced. But the problem is
> that when new interfaces added, all targets dir have to be touched, i.e,
> inserted with useless garbage stuff. These unrelated stuff worse the
> maintain.
>
> b.) "weak" function
> This was proposed by your previously post, and practiced by my last try.
>
> void __attribute__((weak)) Target_Specific_ZDLBR_Expansion(TN * target_tn) {
> }
>
> However, as Jianxin pointed out, this scheme introduce the compiler
> build portability issue. We could not always assume the build compiler
> is gnu.
>
> c.) #ifdef "feature_TARG"
> this is my last approach. Yes, we add more #ifdefs, but the macro is
> defined on the feature rather than the hardware targets. Only enabled
> targets will define "ZDL_TARG" in the build system, then it gains the
> common code for the feature, only at the cost of implementing specific
> interfaces to get work done.
> Compared with the above approaches, I personally think it is a better
> solution.
>
> I'm open to change to general agreement.
>
> Thanks
> Gang
>
> On 09/07/2011 07:44 AM, Sun Chan wrote:
>> I don't like the ZDL_TARGET ifdef. As is, we have too many #ifdefs.
>> Can you use a different scheme?
>> Sun
>>
>> On Tue, Sep 6, 2011 at 4:06 PM, Gang<yugang...@gmail.com> wrote:
>>> Hi, JianXin:
>>>
>>> Thanks very much for the review. A revised patch please find in the
>>> attach.
>>>
>>> On 09/06/2011 05:52 PM, Jian-Xin Lai wrote:
>>>> Hi Gang,
>>>>
>>>> Sorry for the delay. I have the following comments on this patch:
>>>>
>>>> 1. osprey/be/cg/cgemit.cxx, changes about "Emit_Phase_Validity_Check", I
>>>> prefer to declare this in cgemit.h and provide fake implementation for
>>>> all
>>>> other platform. FmtAssert FALSE can be used in the fake implementation.
>>>> The
>>>> same problem about "Target_Specific_ZDLBR_Expansion" in
>>>> osprey/be/cg/whirl2ops.cxx. The major reason is "attribute weak" is not
>>>> very
>>>> portable.
>>>>
>>> Yes. Both for portability and the intension of keeping unrelated
>>> targets
>>> clean, I have changes like the below:
>>>
>>>
>>> +#ifdef ZDL_TARGET
>>> + case OPC_ZDLBR:
>>> + target_tn = Gen_Label_TN (Get_WN_Label (branch), 0);
>>> + /* Target specific Expansion of OPR_ZDLBR */
>>> + extern void Target_Specific_ZDLBR_Expansion(TN* target_tn);
>>> + Target_Specific_ZDLBR_Expansion(target_tn);
>>> + break;
>>> +#endif
>>>
>>> Thus, for ZDL targets, people have to define the
>>> "Target_Specific_ZDL_Expansion" in the target specific files. For non-ZDL
>>> targets, the code remains untouched.
>>>> 2. About TOP_auxbr, is it possible to remove it from the OP_branch or
>>>> OP_cond group in SL's targinfo? So that you don't need to add special
>>>> code
>>>> to check that TOP.
>>>>
>>> No. Since auxbr is expanded for OPR_ZDLBR, to simulate the *loop*
>>> operator, so we have to setup its branch and cond properties. And it is
>>> because of this, I add some target-specific teaching code in cflow.cxx
>>> and
>>> target-specific cgtarget.cxx.
>>>
>>>> 3. In osprey/common/com/opcode_gen_core.h, since OPR_ZDLBR is added for
>>>> all targets, why not move it out of the #ifdef and change the value for
>>>> the
>>>> later OPRs?
>>>>
>>> This is mainly for maintainance, we can add it as the number of last
>>> commonly shared operators. But the operator sequence of the
>>> target-specific
>>> operators are changed. So, wopt may not dump the right output for
>>> pre-built
>>> intermediates.
>>>> 4. In osprey/common/com/SL/targ_const.cxx, I think Yong-Chong has fixed
>>>> this assertion, the change should be removed from the patch.
>>>>
>>> Yes, his patch does better job. I have removed my change.
>>>> 5. In osprey/common/com/SL/config_targ.h, why not define this macro in
>>>> the
>>>> makefile for SL?
>>> Good idea. thanks. It makes the config_targ.h cleaner.
>>>> 6. It looks like there is no options to skip some of the ZDL
>>>> transformation? is it only controlled by the pragma?
>>>>
>>> For ZDL transformation , we have "WOPT_Enable_ZDL" option to control
>>> the
>>> whole zdl transformation. If this is set to on, we assume all
>>> *trip-countable* loops to be transformed to zdl. People may find some
>>> loops
>>> do not need such a transform, they are provided with the zdl off pragma.
>>> We
>>> could not do the reverse, i.e, disable all transformation and using
>>> pragma
>>> zdl on to enable some, since zdl has to be judged by compiler not by
>>> human
>>> being.
>>>> 7. There are some format issues, especially about the space, tab stop,
>>>> coding style, etc.
>>>> For example, line 224, 271, 406, 425, 446~448, 1602, 2265, 2514, 2526,
>>>> 2582 etc
>>>>
>>> Thanks, please find the change in the attached new patch.
>>>
>>> Gang
>>>
>>> ------------------------------------------------------------------------------
>>> Malware Security Report: Protecting Your Business, Customers, and the
>>> Bottom Line. Protect your business and customers by understanding the
>>> threat from malware and how it can impact your online business.
>>> http://www.accelacomm.com/jaw/sfnl/114/51427462/
>>> _______________________________________________
>>> Open64-devel mailing list
>>> Open64-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/open64-devel
>>>
>>>
>
>
------------------------------------------------------------------------------
Using storage to extend the benefits of virtualization and iSCSI
Virtualization increases hardware utilization and delivers a new level of
agility. Learn what those decisions are and how to modernize your storage
and backup environments for virtualization.
http://www.accelacomm.com/jaw/sfnl/114/51434361/
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
