---------- Forwarded message ---------- From: Yiran Wang <yiran.w...@gmail.com> Date: Fri, Sep 10, 2010 at 4:25 PM Subject: Re: [Open64-devel] code review for fix of bug 633 To: Sun Chan <sun.c...@gmail.com>
This chunk is where the variable ret_mtype is first defined. It looks like if the result is needed, it is set in someway, else it is MTYPE_V. 7822 if (need_result) { 7823 if (!WGEN_Keep_Zero_Length_Structs && 7824 TY_mtype (ty_idx) == MTYPE_M && 7825 TY_size (ty_idx) == 0) { 7826 // zero length struct return 7827 ret_mtype = MTYPE_V; 7828 } 7829 else 7830 ret_mtype = TY_mtype (ty_idx); 7831 #ifdef KEY 7832 // If the type must be returned in memory, create a symbol and pass 7833 // its address as the first param. 7834 if (TY_return_in_mem (ty_idx)) { 7835 ret_mtype = MTYPE_V; 7836 return_in_mem = TRUE; 7837 num_args++; // first param is address of return symbol 7838 if (gs_tree_code(exp) == GS_AGGR_INIT_EXPR && !target_wn) 7839 { 7840 // bug 11169: Use the slot to set up the target. 7841 gs_t slot = gs_tree_operand(exp,2); 7842 FmtAssert (gs_tree_code(slot) == GS_VAR_DECL, 7843 ("WGEN_Expand_Expr: Expected VAR_DECL for " 7844 "AGGR_INIT_EXPR slot")); 7845 ST * target_st = Get_ST(slot); 7846 target_wn = WN_Lda (Pointer_Mtype, 0, target_st, 0); 7847 } 7848 } 7849 #endif 7850 } 7851 else 7852 ret_mtype = MTYPE_V; While, the following chunks show how it is used for intrinsics. For SIN intrinsic, it is set to F8, if it is MTYPE_V, and then select corresponding implementation of SIN, by ret_mtype. 8151 case GSBI_BUILT_IN_SQRT: 8152 case GSBI_BUILT_IN_SQRTF: 8153 #ifndef TARG_MIPS // MIPS needs quad emulation for sqrt operation 8154 case GSBI_BUILT_IN_SQRTL: 8155 #endif 8156 if (ret_mtype == MTYPE_V) ret_mtype = MTYPE_F8; 8157 if (! gs_flag_errno_math(program)) { 8158 arg_wn = WGEN_Expand_Expr (gs_tree_value (gs_tree_operand (exp, 1))); 8159 wn = WN_CreateExp1 (OPR_SQRT, ret_mtype, MTYPE_V, arg_wn); 8160 } 8161 else wn = WGEN_Expand_Math_Errno_Sqrt(exp, ty_idx, ret_mtype); 8162 whirl_generated = TRUE; 8163 break; 8164 8165 #ifdef KEY 8166 case GSBI_BUILT_IN_SINF: 8167 #endif 8168 case GSBI_BUILT_IN_SIN: 8169 intrinsic_op = TRUE; 8170 #ifdef TARG_X8664 8171 if (!Force_IEEE_Comparisons) 8172 { 8173 iopc = INTRN_SINL; 8174 if (ret_mtype != MTYPE_F10) 8175 { 8176 // generate a cvt to 'cvt_to' 8177 cvt_to = ret_mtype; 8178 ret_mtype = MTYPE_F10; 8179 } 8180 break; 8181 } 8182 #endif 8183 #ifdef KEY // bug 11305 8184 if (ret_mtype == MTYPE_V) ret_mtype = MTYPE_F8; 8185 #endif 8186 if (ret_mtype == MTYPE_F4) iopc = INTRN_F4SIN; 8187 else if (ret_mtype == MTYPE_F8) iopc = INTRN_F8SIN; 8188 else Fail_FmtAssertion ("unexpected mtype for intrinsic 'sin'"); 8189 break; 8190 And, some more similar code with comments. 8310 // bug 3390 8311 // If return type is void, generate an intrinsic assuming 8312 // double (so if it is without side-effects, optimizer can 8313 // remove it) 8314 if (ret_mtype == MTYPE_V) ret_mtype = MTYPE_F8; 8315 8316 if (ret_mtype == MTYPE_F4) iopc = INTRN_F4EXP; 8317 else if (ret_mtype == MTYPE_F8) iopc = INTRN_F8EXP; 8318 else Fail_FmtAssertion ("unexpected mtype for intrinsic 'exp'"); 8319 intrinsic_op = TRUE; 8320 break; 8321 8322 case GSBI_BUILT_IN_POW: 8323 case GSBI_BUILT_IN_POWF: 8324 // Bug 8195: If for whatever reason the pow(3) call is unused, 8325 // need_result will be false. Then, the value that this very 8326 // function assigns to ret_mtype for pow(3) is MTYPE_V. So, 8327 // just like we handle BUILT_IN_EXP above, we need to reassign 8328 // ret_mtype to MTYPE_F8. 8329 8330 // Note that since pow[lf](3) are not builtin's (unlike the way 8331 // exp[lf]?(3)'s are), we only permit ret_mtype MTYPE_F8 here. I think the result of sqrt is needed for sure, but for the reason I mentioned in previous message, the compiler would not set it TRUE. The simple reason may be it is unable to set a proper value for another parameter ty_idx. (It should be noted that need_result and ty_idx are parameters of the function WGEN_Expand_Expr) 5702 WN * 5703 WGEN_Expand_Expr (gs_t exp, 5704 bool need_result, 5705 TY_IDX nop_ty_idx, 5706 TY_IDX component_ty_idx, 5707 INT64 component_offset, 5708 UINT32 field_id , 5709 bool is_bit_field, 5710 bool is_aggr_init_via_ctor, 5711 WN *target_wn) Best Regards, yiran On Fri, Sep 10, 2010 at 3:35 PM, Sun Chan <sun.c...@gmail.com> wrote: > Could you show larger portion of the code around it. The variable is > ret_mtype, so may be it is only for return type. For a single > statement, the result type usually follows that of the use. > Sun > > On Sat, Sep 11, 2010 at 6:18 AM, Yiran Wang <yiran.w...@gmail.com> wrote: > > The simple reason may be F8 is the largest type for sqrt. > > Another solution may be to set the type to the operand. But I simply want > to > > make it consistent to other intrinsics with similar issue (they solve it > in > > this way). > > > > Best Regards, > > yiran > > > > On Fri, Sep 10, 2010 at 3:14 PM, Sun Chan <sun.c...@gmail.com> wrote: > >> > >> why is F8 the default, but not others? > >> Sun > >> > >> On Sat, Sep 11, 2010 at 6:08 AM, Yiran Wang <yiran.w...@gmail.com> > wrote: > >> > Hi Sun, > >> > Sure. > >> > The situation is as following. > >> > There is implementation of the intrinsic "sqrt", for a number of > types, > >> > such > >> > as F4, F8, and so on. While, the compiler want to determine the exact > >> > type > >> > of the sqrt by context. For example, > >> > double a, b; > >> > a = sqrt(b); > >> > Then the type is F8. > >> > Usually it works, but for the following code: > >> > double x; > >> > int y; > >> > y = sqrt(x*x)>1?1:0; > >> > The result of sqrt is used by the > operator. While, the operator > > >> > would > >> > like to promote the type of the two operands, so they match. So it > would > >> > not > >> > provide type information for the sqrt intrinsic. So, the compiler > simply > >> > generate something like > >> > VSTID... > >> > VSQRT ... > >> > The change is to set default type to F8, for such case. > >> > Best Regards, > >> > yiran > >> > > >> > On Fri, Sep 10, 2010 at 2:54 PM, Sun Chan <sun.c...@gmail.com> wrote: > >> >> > >> >> please give a brief description of the bug. Or a simple test case. > The > >> >> portion of code change is not enough for me to tell why F8 is the > >> >> right type, but not, say, F4, or V8.... > >> >> Sun > >> >> > >> >> On Sat, Sep 11, 2010 at 2:58 AM, Yiran Wang <yiran.w...@gmail.com> > >> >> wrote: > >> >> > Hi All, > >> >> > Could a gatekeeper please review it? > >> >> > Simply, the ret_mtype should be reset automatically, if it is void. > >> >> > This > >> >> > is > >> >> > already done for a number of other intrinsics. > >> >> > The bug is triggered when the combination of > and ?: operator is > >> >> > used, > >> >> > as > >> >> > the result type is not set by parent of the intrinsic. > >> >> > Best Regards, > >> >> > yiran > >> >> > > >> >> > Index: wgen_expr.cxx > >> >> > =================================================================== > >> >> > --- wgen_expr.cxx (revision 3338) > >> >> > +++ wgen_expr.cxx (working copy) > >> >> > @@ -8153,6 +8153,7 @@ > >> >> > #ifndef TARG_MIPS // MIPS needs quad emulation for sqrt operation > >> >> > case GSBI_BUILT_IN_SQRTL: > >> >> > #endif > >> >> > + if (ret_mtype == MTYPE_V) ret_mtype = MTYPE_F8; > >> >> > if (! gs_flag_errno_math(program)) { > >> >> > arg_wn = WGEN_Expand_Expr (gs_tree_value > >> >> > (gs_tree_operand > >> >> > (exp, 1))); > >> >> > wn = WN_CreateExp1 (OPR_SQRT, ret_mtype, MTYPE_V, > >> >> > arg_wn); > >> >> > > >> >> > > >> >> > > >> >> > > ------------------------------------------------------------------------------ > >> >> > Start uncovering the many advantages of virtual appliances > >> >> > and start using them to simplify application deployment and > >> >> > accelerate your shift to cloud computing > >> >> > http://p.sf.net/sfu/novell-sfdev2dev > >> >> > > >> >> > _______________________________________________ > >> >> > Open64-devel mailing list > >> >> > Open64-devel@lists.sourceforge.net > >> >> > https://lists.sourceforge.net/lists/listinfo/open64-devel > >> >> > > >> >> > > >> > > >> > > > > > >
------------------------------------------------------------------------------ Start uncovering the many advantages of virtual appliances and start using them to simplify application deployment and accelerate your shift to cloud computing http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________ Open64-devel mailing list Open64-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/open64-devel