[Bug tree-optimization/94566] conversion between std::strong_ordering and int
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94566 --- Comment #13 from Andrew Macleod --- (In reply to Andrew Pinski from comment #12) > Aldy or Andrew, why in conv1 we don't get a range for > SR.4_4 = sD.8798._M_valueD.7665; > > Even though the range we have is [-1,1] according to the > __builtin_unreachable()? > It seems like we should get that range. Once we do get that the code works. > E.g. If we add: > signed char *t = (signed char*) > signed char tt = *t; > if (tt < -1 || tt > 1) __builtin_unreachable(); > > In the front before the other ifs, we get the code we are expecting. > > conv2 has a similar issue too, though it has also a different issue of > ordering for the comparisons. its because the unreachable is after the branches, and we have multiple uses of SR.4_4 before the unreachable. [local count: 1073741824]: SR.4_4 = s._M_value; if (SR.4_4 == -1) goto ; [50.00%] else goto ; [50.00%] [local count: 536870913]: if (SR.4_4 == 0) goto ; [50.00%] else goto ; [50.00%] [local count: 268435456]: if (SR.4_4 == 1) goto ; [100.00%] else goto ; [0.00%] [count: 0]: __builtin_unreachable (); [local count: 1073741824]: # _1 = PHI <-1(2), 0(3), 1(4)> We know when we get to bb5 that SR.4_4 is [-1, 1] for sure. But we dont know that before we reach that spot. if there was a call to foo(SR.4_4) in bb 3 for instance, we wouldn't be able to propagate [-1,1] to the call to foo because it happens before we know for sure.and foo may go and do something if it has a value of 6 and exit the compilation, thus never returning. So we can only provide a range of [-1, 1] AFTER the unreachable, or if there is only a single use of it.. the multiples uses are what tricks it. This has come up before. we need some sort of backwards propagation that can propagate discovered values earlier into the IL to a point where it is known to be safe (ie, it wouldnt be able to propagate it past a call to foo() for instance) In cases like this, we could discover it is safe to propagate that range back to the def point, and then we could set the global. Until we add some smarts, either to the builtin unreachable elimination code, or elsewhere, which is aware of how to handle such side effects, we can't set the global because we dont know if it is safe at each use before the unreachable call.
[Bug tree-optimization/94566] conversion between std::strong_ordering and int
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94566 Andrew Pinski changed: What|Removed |Added CC||aldyh at gcc dot gnu.org, ||amacleod at redhat dot com --- Comment #12 from Andrew Pinski --- Aldy or Andrew, why in conv1 we don't get a range for SR.4_4 = sD.8798._M_valueD.7665; Even though the range we have is [-1,1] according to the __builtin_unreachable()? It seems like we should get that range. Once we do get that the code works. E.g. If we add: signed char *t = (signed char*) signed char tt = *t; if (tt < -1 || tt > 1) __builtin_unreachable(); In the front before the other ifs, we get the code we are expecting. conv2 has a similar issue too, though it has also a different issue of ordering for the comparisons.
[Bug tree-optimization/94566] conversion between std::strong_ordering and int
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94566 --- Comment #11 from Jakub Jelinek --- (In reply to Oliver Schönrock from comment #10) > I agree the switch optimisation is better, but... > > shouldn't std::bit_cast prevent incorrect casting with different underlying > implementaion? (ie if the size doesn't match, and the size could be deduced > with TMP) The size can be deduced, yes. What the bits actually mean can't be. > and "unordered value" doesn't apply to std::strong_ordering? Sure, but this PR isn't just about strong_ordering, same problem applies for partial_ordering. And actually not just those, but any case of some set of enumerators or macros where you don't know the values exactly and mapping them to or from a set of integer constants, ideally with 1:1 mapping but not guaranteed that way.
[Bug tree-optimization/94566] conversion between std::strong_ordering and int
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94566 --- Comment #10 from Oliver Schönrock --- I agree the switch optimisation is better, but... shouldn't std::bit_cast prevent incorrect casting with different underlying implementaion? (ie if the size doesn't match, and the size could be deduced with TMP) and "unordered value" doesn't apply to std::strong_ordering?
[Bug tree-optimization/94566] conversion between std::strong_ordering and int
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94566 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #9 from Jakub Jelinek --- That one isn't portable, relies on both the strong_ordering underlying implementation using an 8-bit integer member rather than something else, and also hardcodes the exact values where in C++ the -1 / 0 / 1 are exposition only and unordered value is -127 rather than what gcc uses (2). By writing a series of ifs or switch one achieves portability and we'd just like to get efficient code if the values the user chose match those used by the implementation.
[Bug tree-optimization/94566] conversion between std::strong_ordering and int
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94566
--- Comment #8 from Oliver Schönrock ---
how about:
#include
#include
#include
int conv3(std::strong_ordering s){
return std::bit_cast(s);
}
std::strong_ordering conv4(int i){
return std::bit_cast(static_cast(i));
}
conv3(std::strong_ordering):
movsbl %dil, %eax
ret
conv4(int):
movl%edi, %eax
ret
https://godbolt.org/z/szP5MGq4T
[Bug tree-optimization/94566] conversion between std::strong_ordering and int
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94566 Martin Liška changed: What|Removed |Added See Also||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=92005 --- Comment #7 from Martin Liška --- > So here is the proposed pipeline: > iftoswitch - change to do it even without an heurstics > sink > switchconv > switchlower - early This one is also not ideal as there might be an optimization that eliminates some of the switch case branches. Note that we have a similar issue PR92005.
[Bug tree-optimization/94566] conversion between std::strong_ordering and int
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94566
--- Comment #6 from Martin Liška ---
(In reply to Marc Glisse from comment #3)
> I thought we had code to recognize a switch that represents a linear
> function, I was hoping that it would kick in with your hoisting patch...
Yep, we have the code but as mentioned the sinking needs to happen first.
Otherwise, you'll see:
g++ pr94566.C -O2 -fdump-tree-switchconv=/dev/stdout -std=gnu++2a
;; Function conv2 (_Z5conv2i, funcdef_no=90, decl_uid=8463, cgraph_uid=59,
symbol_order=88)
beginning to process the following SWITCH statement (pr94566.C:4) : ---
switch (i_2(D)) [INV], case -1: [INV], case 0: [INV],
case 1: [INV]>
Bailing out - bad case - a non-final BB not empty
struct strong_ordering conv2 (int i)
{
struct strong_ordering D.8476;
:
switch (i_2(D)) [INV], case -1: [INV], case 0:
[INV], case 1: [INV]>
:
:
D.8476._M_value = -1;
goto ; [INV]
:
:
D.8476._M_value = 0;
goto ; [INV]
:
:
D.8476._M_value = 1;
goto ; [INV]
:
:
__builtin_unreachable ();
:
return D.8476;
}
[Bug tree-optimization/94566] conversion between std::strong_ordering and int
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94566 Martin Liška changed: What|Removed |Added CC||marxin at gcc dot gnu.org --- Comment #5 from Martin Liška --- (In reply to Richard Biener from comment #2) > I've verified my old fix for PR33315 does Can you please show me which patch from the PR does that? > > still not optimized further tho. When does the sinking happen in the optimization pipeline?
[Bug tree-optimization/94566] conversion between std::strong_ordering and int
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94566 Andrew Pinski changed: What|Removed |Added Last reconfirmed|2020-04-13 00:00:00 |2021-6-14 --- Comment #4 from Andrew Pinski --- I wonder if we should always convert if-cases to switches and then do switchconv and then lower the smaller ones (3 if max) right after switchconv and then lower the rest in late as we do right now. This should get the conv1 case. The conv2 case problem is don't sink until late and never redo switchconv. So here is the proposed pipeline: iftoswitch - change to do it even without an heurstics sink switchconv switchlower - early
[Bug tree-optimization/94566] conversion between std::strong_ordering and int
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94566 Bug 94566 depends on bug 33315, which changed state. Bug 33315 Summary: stores not commoned by sinking https://gcc.gnu.org/bugzilla/show_bug.cgi?id=33315 What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED
[Bug tree-optimization/94566] conversion between std::strong_ordering and int
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94566 --- Comment #3 from Marc Glisse --- I thought we had code to recognize a switch that represents a linear function, I was hoping that it would kick in with your hoisting patch...
[Bug tree-optimization/94566] conversion between std::strong_ordering and int
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94566
Richard Biener changed:
What|Removed |Added
Blocks|11832, 33315|
Depends on||33315
--- Comment #2 from Richard Biener ---
I've verified my old fix for PR33315 does
t.C:11:45: optimized: sinking common stores to D.8528._M_value
...
conv2 (int i)
{
struct strong_ordering D.8528;
int _7;
[local count: 1073741817]:
switch (i_2(D)) [0.00%], case -1: [33.33%], case 0:
[33.33%], case 1: [33.33%]>
[local count: 357913942]:
:
goto ; [100.00%]
[local count: 357913942]:
:
goto ; [100.00%]
[count: 0]:
:
__builtin_unreachable ();
[local count: 1073741824]:
# _7 = PHI <-1(2), 0(3), 1(4)>
:
D.8528._M_value = _7;
return D.8528;
still not optimized further tho.
Referenced Bugs:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=11832
[Bug 11832] Optimization of common stores in switch statements
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=33315
[Bug 33315] stores not commoned by sinking
[Bug tree-optimization/94566] conversion between std::strong_ordering and int
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94566
--- Comment #1 from Richard Biener ---
So for conv2 the most immediate issue is that we're failing to sink and common
the assignment to D.8516._M_value (I had patches for this).
conv2 (int i)
{
int i_2(D) = i;
struct strong_ordering D.8516;
[local count: 1073741824]:
if (i_2(D) == 0)
goto ; [33.33%]
else
goto ; [66.67%]
[local count: 1073741824]:
if (i_2(D) == 1)
goto ; [50.00%]
else
goto ; [50.00%]
[local count: 357878150]:
D.8516._M_value = -1;
goto ; [100.00%]
[local count: 357878150]:
D.8516._M_value = 0;
goto ; [100.00%]
[local count: 357878150]:
D.8516._M_value = 1;
[local count: 1073634451]:
return D.8516;
for conv1 this is a missed phi-opt, at phiopt2 time:
conv1 (struct strong_ordering s)
{
int SR.4;
int _1;
[local count: 1073741824]:
SR.4_4 = s._M_value;
if (SR.4_4 == -1)
goto ; [50.00%]
else
goto ; [50.00%]
[local count: 536870913]:
if (SR.4_4 == 0)
goto ; [50.00%]
else
goto ; [50.00%]
[local count: 268435456]:
if (SR.4_4 == 1)
goto ; [100.00%]
else
goto ; [0.00%]
[count: 0]:
__builtin_unreachable ();
[local count: 1073741824]:
# RANGE [-1, 1]
# _1 = PHI <-1(2), 0(3), 1(4)>
return _1;
it's a bit of a convoluted case of course but the only chance to pattern
match this ...
[Bug tree-optimization/94566] conversion between std::strong_ordering and int
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94566 Jonathan Wakely changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2020-04-13 See Also||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=94006 Ever confirmed|0 |1
