On 11/02/2015 05:35 PM, Nathan Sidwell wrote:
+/* Size of buffer needed for worker reductions. This has to be
Maybe "description" rather than "Size" since there's really four
variables we're covering with the comment.
+ worker_red_size = (worker_red_size + worker_red_align - 1)
+ & ~(worker_red_align - 1);
Formatting. Wrap the entire multi-line expression in parentheses to get
editors to align the & operator.
+static rtx
+nvptx_expand_cmp_swap (tree exp, rtx target,
+ machine_mode ARG_UNUSED (m), int ARG_UNUSED (ignore))
Add a comment. You're using ATTRIBUTE_UNUSED and ARG_UNUSED in this
patch, it would be good to be consistent - I'm still not sure which
style is preferred after the switch to C++, so as far as I'm concerned
just pick one.
+{
+#define DEF(ID, NAME, T) \
+ (nvptx_builtin_decls[NVPTX_BUILTIN_ ## ID] = \
+ add_builtin_function ("__builtin_nvptx_" NAME, \
+ build_function_type_list T, \
+ NVPTX_BUILTIN_ ## ID, BUILT_IN_MD, NULL, NULL))
I think the assignment operator should start the line like all others,
but other code in gcc is pretty inconsistent in that department.
+static tree
+nvptx_get_worker_red_addr (tree type, tree offset)
Add a comment.
+ switch (TYPE_MODE (TREE_TYPE (var)))
+ {
+ case SFmode:
+ code = VIEW_CONVERT_EXPR;
+ /* FALLTHROUGH */
+ case SImode:
+ break;
+
+ case DFmode:
+ code = VIEW_CONVERT_EXPR;
+ /* FALLTHROUGH */
+ case DImode:
+ type = long_long_unsigned_type_node;
+ fn = NVPTX_BUILTIN_CMP_SWAPLL;
+ break;
+
+ default:
+ gcc_unreachable ();
+ }
There are two such switch statements, and it's possible to write this
more compactly:
if (!INTEGRAL_MODE_P (...))
code = VIEW_CONVERT_EXPR;
if (GET_MODE_SIZE (...) == 8)
fn = CMP_SWAPLL;
Not required, you can decide which you like better.
Otherwise I have no objections.
Bernd
