---------- 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

Reply via email to