This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch, Fortran] Allocate + CAF library


On 07/15/2011 10:03 AM, Tobias Burnus wrote:
+ /* ERRMSG= */
+ errmsg = null_pointer_node;
+ errlen = build_int_cst (gfc_charlen_type_node, 0);
+ if (code->expr2)
+ {
[...]
+ errlen = gfc_get_expr_charlen (code->expr2);
+ errmsg = gfc_build_addr_expr (pchar_type_node, se.expr);
+ }

While build_int_cst is not terribly expensive, it also does not come for free (cf. tree.c); thus, please move the code from before the "if" into an "else".

Ok. Fixed.



+ /* GOTO destinations. */
+ label_errmsg = gfc_build_label_decl (NULL_TREE);
+ label_finish = gfc_build_label_decl (NULL_TREE);

There seems to be a goto missing. For


integer, allocatable :: AA, BB[:], CC
integer :: stat
allocate(CC, AA, stat=stat)

one gets (-fdump-tree-original):

cc = D.1563; /* end of allocation of "CC". */
if (stat.0 != 0) goto L.1;

if ((logical(kind=4)) __builtin_expect (aa != 0B, 0))
....
else
/* Allocate "AA". */

If you try
allocate(BB[*], AA, stat=stat)
instead you do not get the "if (stat.0 != 0) goto L.1;"

Or in English: Assuming one has stat=variable: If you do not have
coarrays, as soon as one allocation fails, one jumps at the end of the
block and the "stat" variable contains a nonzero value.
If the coarray allocation fails, one continues with other allocations
and thus may end up with "stat == 0" although (at least) one (coarray)
allocation has failed.


This is strange. The problem is definitely in the following if branch in gfc_trans_array:

  if (code->expr1 || code->expr2)
    {
      /* The coarray library already sets the errmsg.  */
      if (gfc_option.coarray == GFC_FCOARRAY_LIB
	  && gfc_expr_attr (expr).codimension)
	tmp = build1_v (GOTO_EXPR, label_finish);
      else
	tmp = build1_v (GOTO_EXPR, label_errmsg);
    ...
    }


You see what I'm trying to do here. If the current expression is a coarray and we are using the coarray lib, the library has already set the errmsg and we do not want to set it again. That's why there are two goto destinations. Schematically, it's like this:


label_errmsg:
    if (stat != 0) *errmsg = "Compiler's default message.";

label_finish:
    if (stat != 0) ... write user's stat variable ...;


I other words, the code example that you posted should have two different GOTO targets. If you are using MPI then BB should be pointing at label_finish and AA should be pointing at label_errmsg. And if you are not using MPI, then both should be pointing at label_errmsg.



I'll need some time to think about why this is not working the way I expect.



+ if (status != NULL_TREE)
+ gfc_add_expr_to_block (block,
+ fold_build2_loc (input_location, MODIFY_EXPR, status_type,
+ status, build_int_cst (status_type, 0)));

Indent is wrong (should be two spaces, is more as a left over from removing the { ... }).

Fixed.



+ This function follows the following pseudo-code:
[...]
+ newmem = _caf_register ( size, regtype, NULL,&stat, NULL, NULL);
+ if (newmem == NULL)
+ {
+ if (!stat requested)
+ runtime_error ("Allocation would exceed memory limit");
+ }
+ return newmem;

The "if (newmem == NULL) part is not present in the patch - an error is already printed in _caf_register and thus the check has been removed. However, the comment has not been updated. Additionally, you could replace the last two NULLs by errmsg/errmsg_len.

Fixed.



+gfc_allocate_using_lib (stmtblock_t * block, tree size, tree status,
+ tree errmsg, tree errlen)
[...]

+ /* Set the optional status variable to zero. */
+ if (status != NULL_TREE)
+ gfc_add_expr_to_block (block,
+ fold_build2_loc (input_location, MODIFY_EXPR, status_type,
+ status, build_int_cst (status_type, 0)));
[...]
+ gfc_add_modify (block, res,
+ fold_convert (prvoid_type_node,
+ build_call_expr_loc (input_location,
+ gfor_fndecl_caf_register, 6,

The "stat" variable is already set in the registering function - no need to set it to zero before the call.

Fixed. I tried to clean up the residual code inherited from allocate_with_status but I missed that part.



Anyway, I'll go think about the GOTOs and figure out what went wrong there...


Cheers,
Daniel.
--
I'm not overweight, I'm undertall.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]