Re: [PATCH] rtl-optimization: ppc backend generates unnecessary signed extension.

[Peter Bergner <bergner@linux.ibm.com>]

On 3/23/23 8:47 AM, Jeff Law wrote:
> On 3/23/23 04:38, Ajit Agarwal wrote:
>>     * ree.cc: Modification for  AND opcode support to eliminate
>>     unnecessary signed extension.
>>     * testsuite/g++.target/powerpc/sext-elim.C: New tests.
> Just a note.  I'll look at this once the trunk is open for gcc-14 development.
> It's really not appropriate for gcc-13.

Hi Jeff, yes, we agree 100% that this is stage1 material!  I'm sorry if
this wasn't clear. 



>>     https://gcc.gnu.org/PR41742
> 
> These are not addressed in the trunk patch, because int c is not initialized
> with registers and for this reason we cannot eliminate them. If we initialize
> int c then zero extension goes away.

I'm sorry that I don't know how REE works.  Why can't it optimize this?
I see in the REE dump:

(insn 20 18 22 3 (set (reg:DI 4 4)
                      (zero_extend:DI (reg:QI 4 4 [orig:120 cD.3556+3 ] [120]))) "pr41742.c":6:41 8 {zero_extendqidi2} (nil))
(call_insn 22 20 41 3 (parallel [
            (set (reg:DI 3 3)
                 (call (mem:SI (symbol_ref:DI ("memset") [flags 0x41]  <function_decl 0x7fff925f8400 __builtin_memset>) [0 memsetD.1196 S4 A8])
                    (const_int 64 [0x40])))
            (use (const_int 0 [0]))
            (clobber (reg:DI 96 lr)) ...

Is there a reason why REE cannot see that our (reg:QI 4) is a param register
and thus due to our ABI, already correctly sign/zero extended?



>>     https://gcc.gnu.org/PR65010
>>     https://gcc.gnu.org/PR82940
>>     https://gcc.gnu.org/PR107949
>>
> 
> My patch fixes these PR's which were not fixed in trunk patch.

Great!  Once this goes is, please include these PR #s in your commit log
and mark the PRs as RESOLVED/FIXED.

That said, I see we don't enable -free at -O2 and above like other
architectures do, so we'll get no benefit without explicitly adding -free:

./gcc/common/config/riscv/riscv-common.cc:    { OPT_LEVELS_2_PLUS, OPT_free, NULL, 1 },
./gcc/common/config/aarch64/aarch64-common.cc:    { OPT_LEVELS_2_PLUS, OPT_free, NULL, 1 },
./gcc/common/config/h8300/h8300-common.cc:    { OPT_LEVELS_2_PLUS, OPT_free, NULL, 1 },
./gcc/common/config/i386/i386-common.cc:    { OPT_LEVELS_2_PLUS, OPT_free, NULL, 1 },
./gcc/common/config/sparc/sparc-common.cc:    { OPT_LEVELS_2_PLUS, OPT_free, NULL, 1 },
./gcc/common/config/alpha/alpha-common.cc:    { OPT_LEVELS_2_PLUS, OPT_free, NULL, 1 },

...maybe we should enable it too (in a separate patch) once yours goes in now
that it will actually do something for us?  Thoughts?

I'll note the docs/man page only mention x86, Aarch64 and Alpha enabling REE at
-O2 and above, but clearly others have been added since, so if we enable REE at
-O2 and above, we should fix that too.

Peter