Now that my g77/gcc assignment is in - here's the Fortran-intrinsics-const patch.

Jeffrey A Law law@cygnus.com
Tue Aug 22 15:04:00 GMT 2000


  In message < 39A2C6CB.5FFBA506@moene.indiv.nluug.nl >you write:
   > 2000-08-22  Toon Moene  <toon@moene.indiv.nluug.nl>
  > 
  > 	* com-rt.def: Adapt macro DEFGFRT to accept CONST boolean.
  > 	* com.c (macro DEFGFRT): Use CONST boolean.
  > 	(ffecom_call_binop_): Choose between call by value
  > 	and call by reference.
  > 	(ffecom_expr_): Use direct calls to (g)libc functions for
  > 	POW_DD, LOG10, (float) MOD.
  > 	(ffecom_make_gfrt_): Add const indication to table of
  > 	intrinsics.
  > 	* com.h (macro DEFGFRT): Use CONST boolean.
  > 	* intrin.def: Adjust DEFIMP definition of LOG10, (float) MOD.
Just minor comments --

I'm going to assume that you've got the right functions marked as
constant and the right semantics for by value/by reference parameters.
You're going to know them far better than I :-)  My comments are just
style nits.

Cheers,
jeff


  > *************** ffecom_call_binop_ (tree fn, ffeinfoKind
  > *** 1938,1943 ****
  >     tree right_length;
  >   
  > !   left_tree = ffecom_arg_ptr_to_expr (left, &left_length);
  > !   right_tree = ffecom_arg_ptr_to_expr (right, &right_length);
  >   
  >     left_tree = build_tree_list (NULL_TREE, left_tree);
  > --- 1949,1963 ----
  >     tree right_length;
  >   
  > !   if (ref)	/* Pass arguments by reference. */
  > !     {
  > !       left_tree = ffecom_arg_ptr_to_expr (left, &left_length);
  > !       right_tree = ffecom_arg_ptr_to_expr (right, &right_length);
  > !     }
  > !   else		/* Pass arguments by value. */
  > !     {
  > !       left_tree = ffecom_arg_expr (left, &left_length);
  > !       right_tree = ffecom_arg_expr (right, &right_length);
  > !     }
  > ! 
Style its.  Comments do not belong at EOL, they belong on lines by
themselves.  Two spaces after the period.  I'd push them onto their
own line just before the assignments to left_tree in each arm.


  > *************** ffecom_expr_ (ffebld expr, tree dest_tre
  > *** 3448,3452 ****
  >   				       FFETARGET_charactersizeNONE,
  >   				       FFEEXPR_contextLET);
  > ! 	    code = FFECOM_gfrtPOW_DD;
  >   	    break;
  >   
  > --- 3470,3477 ----
  >   				       FFETARGET_charactersizeNONE,
  >   				       FFEEXPR_contextLET);
  > ! /*	    code = FFECOM_gfrtPOW_DD;
  > ! 	    ref  = TRUE;		*/
  > ! 	    code = FFECOM_gfrtL_POW;
  > ! 	    ref  = FALSE;		/* Pass arguments by value. */
  >   	    break;
Move the comment to the line above ref = FALSE.

Don't comment out code, delete it.  If you feel the need to have
historical info, describe it in english in a comment.

  > *************** ffecom_expr_intrinsic_ (ffebld expr, tre
  > *** 4319,4325 ****
  >   
  >         if (kt == FFEINFO_kindtypeREAL1)
  > ! 	gfrt = FFECOM_gfrtALOG10;
  >         else if (kt == FFEINFO_kindtypeREAL2)
  > ! 	gfrt = FFECOM_gfrtDLOG10;
  >         break;
  >   
  > --- 4345,4353 ----
  >   
  >         if (kt == FFEINFO_kindtypeREAL1)
  > ! /*	gfrt = FFECOM_gfrtALOG10; */
  > ! 	gfrt = FFECOM_gfrtL_LOG10;
  >         else if (kt == FFEINFO_kindtypeREAL2)
  > ! /*	gfrt = FFECOM_gfrtDLOG10; */
  > ! 	gfrt = FFECOM_gfrtL_LOG10;
  >         break;
Similarly.

  >   
  > *************** ffecom_expr_intrinsic_ (ffebld expr, tre
  > *** 4383,4389 ****
  >   
  >         if (kt == FFEINFO_kindtypeREAL1)
  > ! 	gfrt = FFECOM_gfrtAMOD;
  >         else if (kt == FFEINFO_kindtypeREAL2)
  > ! 	gfrt = FFECOM_gfrtDMOD;
  >         break;
  >   
  > --- 4411,4419 ----
  >   
  >         if (kt == FFEINFO_kindtypeREAL1)
  > ! /*	gfrt = FFECOM_gfrtAMOD; */
  > ! 	gfrt = FFECOM_gfrtL_FMOD;
  >         else if (kt == FFEINFO_kindtypeREAL2)
  > ! /*	gfrt = FFECOM_gfrtDMOD; */
  > ! 	gfrt = FFECOM_gfrtL_FMOD;
  >         break;
Similarly.

  > *************** ffecom_make_gfrt_ (ffecomGfrt ix)
  > *** 7065,7070 ****
  > --- 7095,7109 ----
  >   		  ttype);
  >     DECL_EXTERNAL (t) = 1;
  > +   TREE_READONLY (t) = ffecom_gfrt_const_[ix] ? 1 : 0;
  >     TREE_PUBLIC (t) = 1;
  >     TREE_THIS_VOLATILE (t) = ffecom_gfrt_volatile_[ix] ? 1 : 0;
  > + 
  > +   /* Sanity check:  A function that's const cannot be volatile. */
  > + 
  > +   assert (ffecom_gfrt_const_[ix] ? !ffecom_gfrt_volatile_[ix] : 1);
  > + 
  > +   /* Sanity check: A function that's const cannot return complex. */
  > + 
  > +   assert (ffecom_gfrt_const_[ix] ? !ffecom_gfrt_complex_[ix] : 1);
  >   
  >     t = start_decl (t, TRUE);
Don't use assert.  Use

  if (can't happen)
    abort ();

Two spaces after the period in your comments.

jeff



More information about the Gcc-patches mailing list