Message ID | a8184925-e417-d61d-7906-2097e12e2bdf@codesourcery.com |
---|---|
State | New |
Headers | show |
Series | [committed,amdgcn] Allow constants in vector extends and truncates | expand |
Andrew Stubbs <ams@codesourcery.com> writes: > This patch changes the operand predicates such that vector constants are > permitted during compilation. This prevents ICEs caused by the compiler > trying to emit such instructions without checking. That sounds like a target-independent bug though. Why didn't we apply the predicates in the normal way? Richard > > The machine instruction does not accept constants, but it is expected > that the RTL optimizers will remove such artefacts before that becomes a > problem, and if not the register allocator will break it out when it > checks the constraints. > > There's not observable change in the testsuite results, with this patch > alone, but the fix is prerequisite to another patch I'm working on. > > Andrew > > Allow constants in amdgcn extends and truncates > > 2019-12-19 Andrew Stubbs <ams@codesourcery.com> > > gcc/ > * config/gcn/gcn-valu.md > (<convop><VEC_ALL1REG_INT_ALT:mode><VEC_ALL1REG_INT_MODE:mode>2<exec>): > Change input predcate to gcn_alu_operand. > (extend<VEC_ALL1REG_INT_ALT:mode><VEC_ALL1REG_INT_MODE:mode>2<exec>): > Likewise. > (truncv64di<mode>2): Likewise. > (truncv64di<mode>2_exec): Likewise. > (<convop><mode>v64di2): Likewise. > (<convop><mode>v64di2_exec): Likewise. > > diff --git a/gcc/config/gcn/gcn-valu.md b/gcc/config/gcn/gcn-valu.md > index 369aae5bfc5..98dc3e0cb5f 100644 > --- a/gcc/config/gcn/gcn-valu.md > +++ b/gcc/config/gcn/gcn-valu.md > @@ -2491,18 +2491,18 @@ > (truncate "trunc")]) > > (define_insn "<convop><VEC_ALL1REG_INT_ALT:mode><VEC_ALL1REG_INT_MODE:mode>2<exec>" > - [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand" "=v") > + [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand" "=v") > (zero_convert:VEC_ALL1REG_INT_MODE > - (match_operand:VEC_ALL1REG_INT_ALT 1 "register_operand" " v")))] > + (match_operand:VEC_ALL1REG_INT_ALT 1 "gcn_alu_operand" " v")))] > "" > "v_mov_b32_sdwa\t%0, %1 dst_sel:<VEC_ALL1REG_INT_MODE:sdwa> dst_unused:UNUSED_PAD src0_sel:<VEC_ALL1REG_INT_ALT:sdwa>" > [(set_attr "type" "vop_sdwa") > (set_attr "length" "8")]) > > (define_insn "extend<VEC_ALL1REG_INT_ALT:mode><VEC_ALL1REG_INT_MODE:mode>2<exec>" > - [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand" "=v") > + [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand" "=v") > (sign_extend:VEC_ALL1REG_INT_MODE > - (match_operand:VEC_ALL1REG_INT_ALT 1 "register_operand" " v")))] > + (match_operand:VEC_ALL1REG_INT_ALT 1 "gcn_alu_operand" " v")))] > "" > "v_mov_b32_sdwa\t%0, sext(%1) src0_sel:<VEC_ALL1REG_INT_ALT:sdwa>" > [(set_attr "type" "vop_sdwa") > @@ -2515,7 +2515,7 @@ > (define_insn_and_split "truncv64di<mode>2" > [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand" "=v") > (truncate:VEC_ALL1REG_INT_MODE > - (match_operand:V64DI 1 "register_operand" " v")))] > + (match_operand:V64DI 1 "gcn_alu_operand" " v")))] > "" > "#" > "reload_completed" > @@ -2536,7 +2536,7 @@ > [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand" "=v") > (vec_merge:VEC_ALL1REG_INT_MODE > (truncate:VEC_ALL1REG_INT_MODE > - (match_operand:V64DI 1 "register_operand" " v")) > + (match_operand:V64DI 1 "gcn_alu_operand" " v")) > (match_operand:VEC_ALL1REG_INT_MODE 2 "gcn_alu_or_unspec_operand" > "U0") > (match_operand:DI 3 "gcn_exec_operand" " e")))] > @@ -2559,9 +2559,9 @@ > (set_attr "length" "4")]) > > (define_insn_and_split "<convop><mode>v64di2" > - [(set (match_operand:V64DI 0 "register_operand" "=v") > + [(set (match_operand:V64DI 0 "register_operand" "=v") > (any_extend:V64DI > - (match_operand:VEC_ALL1REG_INT_MODE 1 "register_operand" " v")))] > + (match_operand:VEC_ALL1REG_INT_MODE 1 "gcn_alu_operand" " v")))] > "" > "#" > "reload_completed" > @@ -2584,12 +2584,12 @@ > (set_attr "length" "12")]) > > (define_insn_and_split "<convop><mode>v64di2_exec" > - [(set (match_operand:V64DI 0 "register_operand" "=v") > + [(set (match_operand:V64DI 0 "register_operand" "=v") > (vec_merge:V64DI > (any_extend:V64DI > - (match_operand:VEC_ALL1REG_INT_MODE 1 "register_operand" " v")) > - (match_operand:V64DI 2 "gcn_alu_or_unspec_operand" "U0") > - (match_operand:DI 3 "gcn_exec_operand" " e")))] > + (match_operand:VEC_ALL1REG_INT_MODE 1 "gcn_alu_operand" " v")) > + (match_operand:V64DI 2 "gcn_alu_or_unspec_operand" "U0") > + (match_operand:DI 3 "gcn_exec_operand" " e")))] > "" > "#" > "reload_completed"
On 19/12/2019 17:39, Richard Sandiford wrote: > Andrew Stubbs <ams@codesourcery.com> writes: >> This patch changes the operand predicates such that vector constants are >> permitted during compilation. This prevents ICEs caused by the compiler >> trying to emit such instructions without checking. > > That sounds like a target-independent bug though. Why didn't we apply > the predicates in the normal way? Sorry, I've just got back to checking this. I've now confirmed that this is not a target independent bug. The code generating the instructions is in expanders elsewhere in this machine description. They don't attempt to fold constants, so we get an ICE when the following vregs pass attempts to recognise the instructions. By relaxing the predicates we can allow the compiler to fold the instructions in the normal way, and keep the other expanders' code straight-forward. Andrew
Allow constants in amdgcn extends and truncates 2019-12-19 Andrew Stubbs <ams@codesourcery.com> gcc/ * config/gcn/gcn-valu.md (<convop><VEC_ALL1REG_INT_ALT:mode><VEC_ALL1REG_INT_MODE:mode>2<exec>): Change input predcate to gcn_alu_operand. (extend<VEC_ALL1REG_INT_ALT:mode><VEC_ALL1REG_INT_MODE:mode>2<exec>): Likewise. (truncv64di<mode>2): Likewise. (truncv64di<mode>2_exec): Likewise. (<convop><mode>v64di2): Likewise. (<convop><mode>v64di2_exec): Likewise. diff --git a/gcc/config/gcn/gcn-valu.md b/gcc/config/gcn/gcn-valu.md index 369aae5bfc5..98dc3e0cb5f 100644 --- a/gcc/config/gcn/gcn-valu.md +++ b/gcc/config/gcn/gcn-valu.md @@ -2491,18 +2491,18 @@ (truncate "trunc")]) (define_insn "<convop><VEC_ALL1REG_INT_ALT:mode><VEC_ALL1REG_INT_MODE:mode>2<exec>" - [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand" "=v") + [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand" "=v") (zero_convert:VEC_ALL1REG_INT_MODE - (match_operand:VEC_ALL1REG_INT_ALT 1 "register_operand" " v")))] + (match_operand:VEC_ALL1REG_INT_ALT 1 "gcn_alu_operand" " v")))] "" "v_mov_b32_sdwa\t%0, %1 dst_sel:<VEC_ALL1REG_INT_MODE:sdwa> dst_unused:UNUSED_PAD src0_sel:<VEC_ALL1REG_INT_ALT:sdwa>" [(set_attr "type" "vop_sdwa") (set_attr "length" "8")]) (define_insn "extend<VEC_ALL1REG_INT_ALT:mode><VEC_ALL1REG_INT_MODE:mode>2<exec>" - [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand" "=v") + [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand" "=v") (sign_extend:VEC_ALL1REG_INT_MODE - (match_operand:VEC_ALL1REG_INT_ALT 1 "register_operand" " v")))] + (match_operand:VEC_ALL1REG_INT_ALT 1 "gcn_alu_operand" " v")))] "" "v_mov_b32_sdwa\t%0, sext(%1) src0_sel:<VEC_ALL1REG_INT_ALT:sdwa>" [(set_attr "type" "vop_sdwa") @@ -2515,7 +2515,7 @@ (define_insn_and_split "truncv64di<mode>2" [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand" "=v") (truncate:VEC_ALL1REG_INT_MODE - (match_operand:V64DI 1 "register_operand" " v")))] + (match_operand:V64DI 1 "gcn_alu_operand" " v")))] "" "#" "reload_completed" @@ -2536,7 +2536,7 @@ [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand" "=v") (vec_merge:VEC_ALL1REG_INT_MODE (truncate:VEC_ALL1REG_INT_MODE - (match_operand:V64DI 1 "register_operand" " v")) + (match_operand:V64DI 1 "gcn_alu_operand" " v")) (match_operand:VEC_ALL1REG_INT_MODE 2 "gcn_alu_or_unspec_operand" "U0") (match_operand:DI 3 "gcn_exec_operand" " e")))] @@ -2559,9 +2559,9 @@ (set_attr "length" "4")]) (define_insn_and_split "<convop><mode>v64di2" - [(set (match_operand:V64DI 0 "register_operand" "=v") + [(set (match_operand:V64DI 0 "register_operand" "=v") (any_extend:V64DI - (match_operand:VEC_ALL1REG_INT_MODE 1 "register_operand" " v")))] + (match_operand:VEC_ALL1REG_INT_MODE 1 "gcn_alu_operand" " v")))] "" "#" "reload_completed" @@ -2584,12 +2584,12 @@ (set_attr "length" "12")]) (define_insn_and_split "<convop><mode>v64di2_exec" - [(set (match_operand:V64DI 0 "register_operand" "=v") + [(set (match_operand:V64DI 0 "register_operand" "=v") (vec_merge:V64DI (any_extend:V64DI - (match_operand:VEC_ALL1REG_INT_MODE 1 "register_operand" " v")) - (match_operand:V64DI 2 "gcn_alu_or_unspec_operand" "U0") - (match_operand:DI 3 "gcn_exec_operand" " e")))] + (match_operand:VEC_ALL1REG_INT_MODE 1 "gcn_alu_operand" " v")) + (match_operand:V64DI 2 "gcn_alu_or_unspec_operand" "U0") + (match_operand:DI 3 "gcn_exec_operand" " e")))] "" "#" "reload_completed"