| Message ID | 3dc25846-73ef-8021-3dd2-3c16a391b6cf@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | rs6000: Support UN[GL][ET] in rs6000_maybe_emit_maxc_minc [PR105002] | expand |
Hi! On Thu, Mar 24, 2022 at 10:00:43AM +0800, Kewen.Lin wrote: > Commit r12-7687 exposed one miss optimization chance in function > rs6000_maybe_emit_maxc_minc, for now it only considers comparison > codes GE/GT/LE/LT, but it can support more variants with codes > UNLT/UNLE/UNGT/UNGE by reversing them into the equivalent ones > with GE/GT/LE/LT. > + /* Canonicalize UN[GL][ET] to [LG][TE]. */ > + if (code == UNGE || code == UNGT || code == UNLE || code == UNLT) > + { > + code = reverse_condition_maybe_unordered (code); > + std::swap (true_cond, false_cond); > + } Typically you would have to generate code to compensate for the reversed comparison. It works out fine here, but could you please restructure the code to make that less tricky / more obvious? Or at least add a comment? I wouldn't call it "canonicalisation" btw, LT is by no means more standard than UNGE is. You can say you are folding things so you later have to support fewer cases, or similar? Thanks, Segher
Hi Segher, on 2022/3/27 11:02 PM, Segher Boessenkool wrote: > Hi! > > On Thu, Mar 24, 2022 at 10:00:43AM +0800, Kewen.Lin wrote: >> Commit r12-7687 exposed one miss optimization chance in function >> rs6000_maybe_emit_maxc_minc, for now it only considers comparison >> codes GE/GT/LE/LT, but it can support more variants with codes >> UNLT/UNLE/UNGT/UNGE by reversing them into the equivalent ones >> with GE/GT/LE/LT. > >> + /* Canonicalize UN[GL][ET] to [LG][TE]. */ >> + if (code == UNGE || code == UNGT || code == UNLE || code == UNLT) >> + { >> + code = reverse_condition_maybe_unordered (code); >> + std::swap (true_cond, false_cond); >> + } > > Typically you would have to generate code to compensate for the reversed > comparison. It works out fine here, but could you please restructure > the code to make that less tricky / more obvious? Or at least add a > comment? > > I wouldn't call it "canonicalisation" btw, LT is by no means more > standard than UNGE is. You can say you are folding things so you later > have to support fewer cases, or similar? > Thanks for the review! Sorry for the late reply (I'm just back from vacation). I just posted v2 by adding more comments to describe the reversions and changing the bad "canonicalisation" word, hope it looks better to you. :) v2: https://gcc.gnu.org/pipermail/gcc-patches/2022-April/592624.html BR, Kewen
diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc index 283e8306ff7..b6a2509788f 100644 --- a/gcc/config/rs6000/rs6000.cc +++ b/gcc/config/rs6000/rs6000.cc @@ -15872,6 +15872,13 @@ rs6000_maybe_emit_maxc_minc (rtx dest, rtx op, rtx true_cond, rtx false_cond) if (result_mode != compare_mode) return false; + /* Canonicalize UN[GL][ET] to [LG][TE]. */ + if (code == UNGE || code == UNGT || code == UNLE || code == UNLT) + { + code = reverse_condition_maybe_unordered (code); + std::swap (true_cond, false_cond); + } + if (code == GE || code == GT) max_p = true; else if (code == LE || code == LT)