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
