On 3/16/23 04:11, Ajit Agarwal via Gcc-patches wrote:
Hello Richard:
On 16/03/23 3:22 pm, Richard Biener wrote:
On Thu, Mar 16, 2023 at 9:19 AM Ajit Agarwal <aagar...@linux.ibm.com> wrote:
On 16/03/23 1:44 pm, Richard Biener wrote:
On Thu, Mar 16, 2023 at 9:11 AM Ajit Agarwal <aagar...@linux.ibm.com> wrote:
Hello Richard:
On 16/03/23 1:10 pm, Richard Biener wrote:
On Thu, Mar 16, 2023 at 6:21 AM Ajit Agarwal via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
Hello All:
This patch eliminates unnecessary zero extension instruction from power
generated assembly.
Bootstrapped and regtested on powerpc64-linux-gnu.
What makes this so special that we cannot deal with it from generic code?
In particular we do have the REE pass, why is target specific
knowledge neccessary
to eliminate the extension?
For returning bool values and comparision with integers generates the following
by all the rtl passes.
set compare (subreg)
set if_then_else
Convert SImode -> QImode
set zero_extend to SImode from QImode
set return value 0 in one path of cfg.
set return value 1 in other path of cfg.
This pass replaces the above zero extension and conversion from QImode to
DImode with copy operation to keep QImode in 64 bit registers in powerpc target.
Sorry, I can't parse that - as there's no testcase with the patch I
cannot even try to see what the actual RTL
looks like (without the pass).
Here is the PR with bugzilla.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784
I can add the attached testcase with this PR in the patch.
I don't see any zero-extends there.
Here is the testcase.
bool (int a, int b)
{
if (a > 2)
return false;
if (b < 10)
return true;
return false;
}
compiled with gcc -O3 -m64 testcase.cc -mcpu=power9 -save-temps.
Here is the rtl after cse.
(note 12 11 15 3 [bb 3] NOTE_INSN_BASIC_BLOCK)
(insn 15 12 16 3 (set (reg:CC 123)
(compare:CC (subreg/s/u:SI (reg/v:DI 120 [ b ]) 0)
(const_int 9 [0x9]))) "ext.cc":5:5 796 {*cmpsi_signed}
(expr_list:REG_DEAD (reg/v:DI 120 [ b ])
(nil)))
(insn 16 15 17 3 (set (reg:SI 124)
(const_int 1 [0x1])) "ext.cc":5:5 555 {*movsi_internal1}
(nil))
(insn 17 16 18 3 (set (reg:SI 122)
(if_then_else:SI (gt (reg:CC 123)
(const_int 0 [0]))
(const_int 0 [0])
(reg:SI 124))) "ext.cc":5:5 344 {isel_cc_si}
(expr_list:REG_DEAD (reg:SI 124)
(expr_list:REG_DEAD (reg:CC 123)
(nil))))
(insn 18 17 32 3 (set (reg:QI 117 [ _1 ])
(subreg:QI (reg:SI 122) 0)) "ext.cc":5:5 562 {*movqi_internal}
(expr_list:REG_DEAD (reg:SI 122)
(nil)))
; pc falls through to BB 5
(code_label 32 18 31 4 3 (nil) [1 uses])
(note 31 32 5 4 [bb 4] NOTE_INSN_BASIC_BLOCK)
(insn 5 31 19 4 (set (reg:QI 117 [ _1 ])
(const_int 0 [0])) "ext.cc":4:16 562 {*movqi_internal}
(nil))
(code_label 19 5 20 5 2 (nil) [0 uses])
(note 20 19 21 5 [bb 5] NOTE_INSN_BASIC_BLOCK)
(insn 21 20 22 5 (set (reg:DI 126 [ _1 ])
(zero_extend:DI (reg:QI 117 [ _1 ]))) "ext.cc":8:1 5 {zero_extendqidi2}
(expr_list:REG_DEAD (reg:QI 117 [ _1 ])
(nil)))
(insn 22 21 26 5 (set (reg:DI 118 [ <retval> ])
(reg:DI 126 [ _1 ])) "ext.cc":8:1 681 {*movdi_internal64}
(expr_list:REG_DEAD (reg:DI 126 [ _1 ])
(nil)))
(insn 26 22 27 5 (set (reg/i:DI 3 3)
(reg:DI 126 [ _1 ])) "ext.cc":8:1 681 {*movdi_internal64}
(expr_list:REG_DEAD (reg:DI 118 [ <retval> ])
(nil)))
(insn 27 26 0 5 (use (reg/i:DI 3 3)) "ext.cc":8:1 -1
(nil))
This looks like it'd be better addressed in REE.
We've got two paths to the zero_extend. One sets (reg 117) from a
constant. The other sets (reg 117) from a (subreg:QI (reg:SI)).
Handling the constant is trivial. For the other set, we can replace the
subreg with the zero_extend. Presumably we'd then proceed to try and
eliminate the zero-extend by realizing both arms of the conditional move
are constants and thus trivially handled.
While I don't think REE would handle all this today, fixing it to handle
this case seems like it'd be better than doing a specialized pass in the
ppc backend.
jeff
