On Thu, Oct 22, 2015 at 08:50:23AM -0400, Nathan Sidwell wrote: > >>+ expr = build2 (TRUNC_MOD_EXPR, ivar_type, ivar, > >>+ fold_convert (ivar_type, collapse->iters)); > >>+ expr = build2 (MULT_EXPR, diff_type, fold_convert (diff_type, expr), > >>+ collapse->step); > >>+ expr = build2 (plus_code, iter_type, collapse->base, > >>+ fold_convert (plus_type, expr)); > > > >Shouldn't these be fold_build2 instead? > > I don't think fold_build2 makes a difference here, as (at least) one operand > is a variable?
Fold does tons of optimizations, some could be relevant even for this case. But if you think build2 is fine, I can live with it. > >>+/* FORK and JOIN mark the points at which OpenACC partitioned > >>+ execution is entered or exited. They take an INTEGER_CST argument, > >>+ indicating the axis of forking or joining and return nothing. */ > >>+#define IFN_UNIQUE_OACC_FORK 1 > >>+#define IFN_UNIQUE_OACC_JOIN 2 > >>+/* HEAD_MARK and TAIL_MARK are used to demark the sequence entering or > >>+ leaving partitioned execution. */ > >>+#define IFN_UNIQUE_OACC_HEAD_MARK 3 > >>+#define IFN_UNIQUE_OACC_TAIL_MARK 4 > > > >Shouldn't these be in an enum, to make debugging easier? > > internal-fn.def can be included multiple times in one file (probably only > internal-fn.c). Thus an enum would either need to go somewhere else (and > I'd like to keep it close to the ifn def), or need to be protected in some > manner. Hence I went with #defs, which are safe to duplicate. Any thoughts > on how to resolve that contention? I think an enum in internal-fn.h is better, the IFN_UNIQUE comment can just say that it uses this and this enum from internal-fn.h and the description go there. Being able to just p (enum ifn_unique_kind) gimple_call_arg (call, 0) is valuable (though, perhaps you could even tweak the gimple-pretty-print.c dumper to dump the first argument to IFN_UNIQUE symbolically instead of numerically. Jakub