On 17-06-19 14:35, Alexander Monakov wrote:
> On Mon, 17 Jun 2019, Tom de Vries wrote:
>
>> Hi Alexander,
>>
>> any comments?
>
> A couple suggestions (see below), but no serious concerns from my side.
>
>> --- a/gcc/config/nvptx/nvptx.c
>> +++ b/gcc/config/nvptx/nvptx.c
>> @@ -112,6 +112,17 @@ enum nvptx_data_area
>> DATA_AREA_MAX
>> };
>>
>> +rtx
>> +gen_set_softstack_insn (rtx op)
>> +{
>
> Here it might be appropriate to have something like
>
> gcc_assert (GET_MODE (op) == Pmode);
>
> because failure to supply a Pmode operand indicates a bug in the caller and
> it may be desirable to catch it here; doesn't make sense to allow restoring
> stack pointer from a 32-bit register with a 64-bit address space.
>
> Likewise for other instances of this 'if (... DImode) ... else if (...
> SImode) ...'
> pattern in the patch.
>
Good point, implemented.
>> + if (GET_MODE (op) == DImode)
>> + return gen_set_softstack_insn_di (op);
>> + else if (GET_MODE (op) == SImode)
>> + return gen_set_softstack_insn_si (op);
>> + else
>> + gcc_unreachable ();
>> +}
>> +
>> /* We record the data area in the target symbol flags. */
>> #define SYMBOL_DATA_AREA(SYM) \
>> (nvptx_data_area)((SYMBOL_REF_FLAGS (SYM) >> SYMBOL_FLAG_MACH_DEP_SHIFT) \
>> diff --git a/gcc/config/nvptx/nvptx.md b/gcc/config/nvptx/nvptx.md
>> index 3ed5296db96..1a676b942b2 100644
>> --- a/gcc/config/nvptx/nvptx.md
>> +++ b/gcc/config/nvptx/nvptx.md
>> @@ -1071,8 +1071,8 @@
>> DONE;
>> })
>>
>> -(define_insn "set_softstack_insn"
>> - [(unspec [(match_operand 0 "nvptx_register_operand" "R")]
>> +(define_insn "set_softstack_insn_<mode>"
>> + [(unspec [(match_operand:P 0 "nvptx_register_operand" "R")]
>
> A purely cosmetic issue, but I'll mention it for completeness: the '_insn'
> suffix is unnecessary with the '_<mode>' suffix added. This is how e.g. the
> i386 backend deals with a similar issue, they have 'stack_protect_test' expand
> and 'stack_protect_test_<mode>' insns.
Updated accordingly, and committed as attached.
Thanks,
- Tom
[nvptx] Fix missing mode warnings in nvptx.md, omp part
Fix these warnings:
...
gcc/config/nvptx/nvptx.md:1074:1: warning: operand 0 missing mode?
gcc/config/nvptx/nvptx.md:1240:1: warning: operand 0 missing mode?
gcc/config/nvptx/nvptx.md:1240:1: warning: operand 1 missing mode?
gcc/config/nvptx/nvptx.md:1240:1: warning: operand 2 missing mode?
gcc/config/nvptx/nvptx.md:1268:1: warning: operand 0 missing mode?
...
Build and reg-tested on x86_64 with nvptx accelerator.
2019-06-17 Tom de Vries <tdevr...@suse.de>
* config/nvptx/nvptx-protos.h (gen_set_softstack_insn): Declare.
* config/nvptx/nvptx.c (gen_set_softstack_insn): New function.
* config/nvptx/nvptx.md (define_insn "set_softstack_insn"): Rename to
...
(define_insn "set_softstack_<mode>"): ... this. Use P iterator on
match_operand 0.
(define_insn "omp_simt_enter_insn"): Rename to ...
(define_insn "omp_simt_enter_<mode>"): ... this. Use P iterator on
match_operand 0, 1 and 2, as well as the unspec_volatile result.
(define_expand "omp_simt_enter): Use gen_omp_simt_enter_di and
gen_omp_simt_enter_si.
(define_expand "omp_simt_exit"): New.
(define_insn "omp_simt_exit"): Rename to ...
(define_insn "omp_simt_exit_<mode>"): ... this. Use P iterator on
match_operand 0.
---
gcc/config/nvptx/nvptx-protos.h | 1 +
gcc/config/nvptx/nvptx.c | 12 ++++++++++++
gcc/config/nvptx/nvptx.md | 38 +++++++++++++++++++++++++++++---------
3 files changed, 42 insertions(+), 9 deletions(-)
diff --git a/gcc/config/nvptx/nvptx-protos.h b/gcc/config/nvptx/nvptx-protos.h
index be09a15e49c..061897a3921 100644
--- a/gcc/config/nvptx/nvptx-protos.h
+++ b/gcc/config/nvptx/nvptx-protos.h
@@ -57,5 +57,6 @@ extern const char *nvptx_output_set_softstack (unsigned);
extern const char *nvptx_output_simt_enter (rtx, rtx, rtx);
extern const char *nvptx_output_simt_exit (rtx);
extern const char *nvptx_output_red_partition (rtx, rtx);
+extern rtx gen_set_softstack_insn (rtx);
#endif
#endif
diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
index c53a1ae9f26..aa4a67fbead 100644
--- a/gcc/config/nvptx/nvptx.c
+++ b/gcc/config/nvptx/nvptx.c
@@ -112,6 +112,18 @@ enum nvptx_data_area
DATA_AREA_MAX
};
+rtx
+gen_set_softstack_insn (rtx op)
+{
+ gcc_assert (GET_MODE (op) == Pmode);
+ if (GET_MODE (op) == DImode)
+ return gen_set_softstack_di (op);
+ else if (GET_MODE (op) == SImode)
+ return gen_set_softstack_si (op);
+ else
+ gcc_unreachable ();
+}
+
/* We record the data area in the target symbol flags. */
#define SYMBOL_DATA_AREA(SYM) \
(nvptx_data_area)((SYMBOL_REF_FLAGS (SYM) >> SYMBOL_FLAG_MACH_DEP_SHIFT) \
diff --git a/gcc/config/nvptx/nvptx.md b/gcc/config/nvptx/nvptx.md
index 3ed5296db96..84c0ea45431 100644
--- a/gcc/config/nvptx/nvptx.md
+++ b/gcc/config/nvptx/nvptx.md
@@ -1071,8 +1071,8 @@
DONE;
})
-(define_insn "set_softstack_insn"
- [(unspec [(match_operand 0 "nvptx_register_operand" "R")]
+(define_insn "set_softstack_<mode>"
+ [(unspec [(match_operand:P 0 "nvptx_register_operand" "R")]
UNSPEC_SET_SOFTSTACK)]
"TARGET_SOFT_STACK"
{
@@ -1237,10 +1237,10 @@
;; Patterns for OpenMP SIMD-via-SIMT lowering
-(define_insn "omp_simt_enter_insn"
- [(set (match_operand 0 "nvptx_register_operand" "=R")
- (unspec_volatile [(match_operand 1 "nvptx_nonmemory_operand" "Ri")
- (match_operand 2 "nvptx_nonmemory_operand" "Ri")]
+(define_insn "omp_simt_enter_<mode>"
+ [(set (match_operand:P 0 "nvptx_register_operand" "=R")
+ (unspec_volatile:P [(match_operand:P 1 "nvptx_nonmemory_operand" "Ri")
+ (match_operand:P 2 "nvptx_nonmemory_operand" "Ri")]
UNSPECV_SIMT_ENTER))]
""
{
@@ -1261,12 +1261,32 @@
cfun->machine->simt_stack_align = MAX (UINTVAL (operands[2]),
cfun->machine->simt_stack_align);
cfun->machine->has_simtreg = true;
- emit_insn (gen_omp_simt_enter_insn (operands[0], operands[1], operands[2]));
+ gcc_assert (GET_MODE (operands[0]) == Pmode);
+ if (GET_MODE (operands[0]) == DImode)
+ emit_insn (gen_omp_simt_enter_di (operands[0], operands[1], operands[2]));
+ else if (GET_MODE (operands[0]) == SImode)
+ emit_insn (gen_omp_simt_enter_si (operands[0], operands[1], operands[2]));
+ else
+ gcc_unreachable ();
+ DONE;
+})
+
+(define_expand "omp_simt_exit"
+ [(match_operand 0 "nvptx_register_operand" "R")]
+ ""
+{
+ gcc_assert (GET_MODE (operands[0]) == Pmode);
+ if (GET_MODE (operands[0]) == DImode)
+ emit_insn (gen_omp_simt_exit_di (operands[0]));
+ else if (GET_MODE (operands[0]) == SImode)
+ emit_insn (gen_omp_simt_exit_si (operands[0]));
+ else
+ gcc_unreachable ();
DONE;
})
-(define_insn "omp_simt_exit"
- [(unspec_volatile [(match_operand 0 "nvptx_register_operand" "R")]
+(define_insn "omp_simt_exit_<mode>"
+ [(unspec_volatile [(match_operand:P 0 "nvptx_register_operand" "R")]
UNSPECV_SIMT_EXIT)]
""
{
