[Bug target/103784] New: suboptimal code for returning bool value on target ppc

[Bug target/103784] New: suboptimal code for returning bool value on target ppc

[guihaoc at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

            Bug ID: 103784
           Summary: suboptimal code for returning bool value on target ppc
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: guihaoc at gcc dot gnu.org
  Target Milestone: ---

//test.c

#include <stdbool.h>

bool foo (int a, int b)
{
  if (a > 2)
    return false;
  if (b < 10)
    return true;
  return true;
}

//assembly with trunk
        ld 9,0(3)
        cmpdi 0,9,0
        add 10,9,4
        beq 0,.L5
        ldarx 8,0,3
        cmpd 0,8,9
        bne 0,.L4
        stdcx. 10,0,3
        bne 0,.L4
        li 3,1
        rldicl 3,3,0,63
        blr
        .p2align 4,,15
.L5:
        li 3,0
        rldicl 3,3,0,63
        blr

//assembly with at13.0
        subfic 3,3,2
        srdi 3,3,63
        xori 3,3,0x1
        blr

The second branch and two zero extend are unnecessary. If it returns a integer,
the code seems good.

//test1.c
int foo (int a, int b)
{
  if (a > 2)
    return 0;
  if (b < 10)
    return 1;
  return 1;
}

//assembly with trunk
        li 9,1
        cmpwi 0,3,2
        isel 3,0,9,1
        blr

[Bug target/103784] suboptimal code for returning bool value on target ppc

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---

        ldarx 8,0,3
        cmpd 0,8,9
        bne 0,.L4
        stdcx. 10,0,3
        bne 0,.L4


Is a compare and exchange, that can't be right? Did you paste the right thing
here?

[Bug target/103784] suboptimal code for returning bool value on target ppc

[guihaoc at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

--- Comment #2 from HaoChen Gui <guihaoc at gcc dot gnu.org> ---
Sorry, I pasted wrong codes. Here are the correct ones.

//test.c
#include <stdbool.h>

bool foo (int a, int b)
{
  if (a > 2)
    return false;
  if (b < 10)
    return true;
  return false;
}

//assembly with the trunk
        cmpwi 0,3,2
        bgt 0,.L3
        cmpwi 0,4,9
        li 3,1
        isel 3,0,3,1
        rldicl 3,3,0,63
        blr
        .p2align 4,,15
.L3:
        li 3,0
        rldicl 3,3,0,63
        blr

The two zero extend (rldicl) are unnecessary?

[Bug target/103784] suboptimal code for returning bool value on target ppc

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I wonder if this is because on the gimple level we are producing:

  _5 = b_3(D) <= 9;
  _6 = a_2(D) <= 2;
  _7 = _5 & _6;

Now.

Can you add -fdump-tree-optimized=/dev/stdout and provide the output of that ?

[Bug target/103784] suboptimal code for returning bool value on target ppc

[guihaoc at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

--- Comment #4 from HaoChen Gui <guihaoc at gcc dot gnu.org> ---
output with "-fdump-tree-optimized=/dev/stdout"
;; Function foo (foo, funcdef_no=0, decl_uid=3317, cgraph_uid=1,
symbol_order=0)

Removing basic block 5
_Bool foo (int a, int b)
{
  _Bool _1;
  _Bool _5;

  <bb 2> [local count: 1073741824]:
  if (a_2(D) > 2)
    goto <bb 4>; [34.00%]
  else
    goto <bb 3>; [66.00%]

  <bb 3> [local count: 708669601]:
  _5 = b_3(D) <= 9;

  <bb 4> [local count: 1073741824]:
  # _1 = PHI <_5(3), 0(2)>
  return _1;

}

[Bug target/103784] suboptimal code for returning bool value on target ppc

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

--- Comment #5 from Segher Boessenkool <segher at gcc dot gnu.org> ---
I get simply

        subfic 3,3,2
        srdi 3,3,63
        xori 3,3,0x1
        blr

for this code.  Did you use -O0 or similar?

[Bug target/103784] suboptimal code for returning bool value on target ppc

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

--- Comment #6 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Ugh, this PR is for GCC 12 only, ignore me please :-)

[Bug target/103784] suboptimal code for returning bool value on target ppc

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

--- Comment #7 from Segher Boessenkool <segher at gcc dot gnu.org> ---
I do get that exact same code with everything from GCC 6 to GCC 12 as well
though (modulo a small regression in GCC 10).

[Bug target/103784] suboptimal code for returning bool value on target ppc

[jskumari at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

--- Comment #8 from Surya Kumari Jangala <jskumari at gcc dot gnu.org> ---
Using -O3 with gcc13, I got (with the test in comment 2):

For P8:
        cmpwi 0,3,2
        bgt 0,.L3
        subfic 4,4,9
        srdi 3,4,63
        xori 3,3,0x1
        rldicl 3,3,0,63
        blr
        .p2align 4,,15
.L3:
        li 3,0
        rldicl 3,3,0,63
        blr


For P10:
        cmpwi 0,3,2
        bgt 0,.L3
        cmpwi 0,4,9
        setbcr 3,1
        rldicl 3,3,0,63
        blr
        .p2align 4,,15
.L3:
        li 3,0
        rldicl 3,3,0,63
        blr

[Bug target/103784] suboptimal code for returning bool value on target ppc

[jskumari at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

--- Comment #9 from Surya Kumari Jangala <jskumari at gcc dot gnu.org> ---
The same issue of unnecessary rldicl instruction is there if we change return
value from bool to int.

int foo (int a, int b)
{
  if (a > 2)
    return 0;
  if (b < 10)
    return 1;
  return 0;
}


        cmpwi 0,3,2
        bgt 0,.L3
        subfic 4,4,9
        srdi 3,4,63
        xori 3,3,0x1
        rldicl 3,3,0,63
        blr
        .p2align 4,,15
.L3:
        li 3,0
        rldicl 3,3,0,63
        blr

[Bug target/103784] suboptimal code for returning bool value on target ppc

[jskumari at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

--- Comment #10 from Surya Kumari Jangala <jskumari at gcc dot gnu.org> ---
After the expand pass, we have a single return bb which first zero extends r117
(this reg holds the return value which has been set by predecessor blocks).
Zero extension is done because r117 is of mode QI and we want to change the
mode to DI. After zero extension, r117 is copied to r3.

The input RTL to the peephole2 pass is similar, ie, the writing of value to r3
occurs in predecessor BBs while zero extension of r3 happens in the return bb.
So we cannot do any peephole optimization to get rid of the unnecessary zero
extension.
Note that when return value is written into r3, it has mode QI. Later in the
return bb, r3 is zero extended to convert it's mode into DI.

However, after the bbro (basic block reordering) pass, we have 2 return BBs.
And in each BB, the return value is copied into r3 (in QI mode), and then r3 is
zero extended. Note that bbro occurs after peephole2.
We can do another peephole after bbro, and get rid of the unnecessary zero
extension.
However, we need not always get an opportunity to do a peephole. That is, the
instructions that write into r3 and zero extend r3 can be in different BBs.

A possible solution to this issue would be to have a separate pass that can
remove the zero extends.

In brief, the new pass can do the following:

Fist create webs.
Then find definitions (that is, writes into registers) that reach zero extend
insns.
Mark such definitions (to indicate that the value is going to be zero extended
later on), and then at the time of assembly generation (final pass),
definitions which have been marked should be converted to assembly instructions
which work on the extended mode (for example, with -m64, the generated assembly
should work on the entire 64bit register instead of just a part of it.).
If we generate such assembly instructions, then the zero extend instruction can
be removed, ie, no assembly need be generated.

Note that for definitions that reach zero extends as well as other uses, we
cannot remove the zero extends.

[Bug target/103784] suboptimal code for returning bool value on target ppc

[dje at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

--- Comment #11 from David Edelsohn <dje at gcc dot gnu.org> ---
Have you looked on the GCC mailing list for zero-extend elimination (zee) and
sign-extend elimination (see)? The many, previous proposals for such passes.

[Bug target/103784] suboptimal code for returning bool value on target ppc

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

--- Comment #12 from Segher Boessenkool <segher at gcc dot gnu.org> ---
What David says :-)

We really could use something good for this, it has been a problem for all
GCC targets since forever; it hurts rs6000 more than most though.

Before RA this is a diamond, one side does the 0/1, the other the always 0.
After the join it gets an AND with 1 (not an extend; the effect is similar
of course).  Shrink-wrapping gets rid of the join (duplicates the tail code
to both branches) but does not optimise the result of that, which gives us
the silly li 3,0;clrldi 3,3,63 (and the other side does not need the shift
either, but doesn't look quite as silly :-) ).

It is not unlikely this would work better if we had no QImode thing for the
bool; even SImode might work better already, but DImode would be best (in
the ABI everything is passed in full registers always, so something has to
set the upper bits somewhere).

[Bug target/103784] suboptimal code for returning bool value on target ppc

[jskumari at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

--- Comment #13 from Surya Kumari Jangala <jskumari at gcc dot gnu.org> ---
Thanks David and Segher for your comments. I wanted to note down my analysis
and thoughts from when I had worked on this bug in January. Ajit is looking
into it now.

[Bug target/103784] suboptimal code for returning bool value on target ppc

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

--- Comment #14 from Richard Biener <rguenth at gcc dot gnu.org> ---
I get, for -mcpu=power9 or power10

foo:
.LFB0:
        .cfi_startproc
        cmpwi 0,3,2
        bgt 0,.L3
        cmpwi 0,4,9
        li 3,1
        isel 3,0,3,1
        blr
        .p2align 4,,15
.L3:
        li 3,0
        blr

and I don't see any zero_extend anywhere.

[Bug target/103784] suboptimal code for returning bool value on target ppc

[aagarwa at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

Ajit Kumar Agarwal <aagarwa at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-04-11
             Status|UNCONFIRMED                 |ASSIGNED

[Bug target/103784] suboptimal code for returning bool value on target ppc

[jskumari at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

--- Comment #15 from Surya Kumari Jangala <jskumari at gcc dot gnu.org> ---
This is another test which has unnecessary zero extension:

#include <stdbool.h>

bool glob1;
bool glob2;

bool foo (int a, bool d)
{
  bool c;
  if (a > 2)
    c = glob1 & glob2;
  else
    c = glob1 | glob2;
  return c^d;
}


I am not sure if this is handled by the patch at
https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624751.html as the RTL for
this test has a different CFG shape than what is mentioned in the patch.

[Bug target/103784] suboptimal code for returning bool value on target ppc

[aagarwa at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103784

--- Comment #16 from Ajit Kumar Agarwal <aagarwa at gcc dot gnu.org> ---
This patch https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624751.html

removes the zero extension from the below testcase that has different cfg, My
patch is not based on any CFG shape but it is general valid for all the CFG.

Testcase from Surya.
#include <stdbool.h>

bool glob1;
bool glob2;

bool foo (int a, bool d)
{
  bool c;
  if (a > 2)
    c = glob1 & glob2;
  else
    c = glob1 | glob2;
  return c^d;
}