public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][WIP] PR tree-optimization/101808 Boolean comparison simplification
@ 2021-11-23 18:34 Navid Rahimi
  2021-11-23 19:14 ` Jeff Law
  0 siblings, 1 reply; 11+ messages in thread
From: Navid Rahimi @ 2021-11-23 18:34 UTC (permalink / raw)
  To: Navid Rahimi via Gcc-patches

[-- Attachment #1: Type: text/plain, Size: 531 bytes --]

Hi GCC community,

I wanted you take a quick look at this patch to solve this bug [1]. This is the code example for the optimization [2] which does include a link to proof of each different optimization.

I think it should be possible to use simpler approach than what Andrew has used here [3].

P.S. Tested and verified on Linux x86_64.

1) https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101808 
2) https://compiler-explorer.com/z/Gc448eE3z
3) https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101808#c1

Best wishes,
Navid.

[-- Attachment #2: 0001-PR-tree-optimization-101808.patch --]
[-- Type: application/octet-stream, Size: 3144 bytes --]

From 30f8a30be575aaa69ccad8c8eec26ac7737c2e78 Mon Sep 17 00:00:00 2001
From: Navid Rahimi <navidrahimi@microsoft.com>
Date: Mon, 22 Nov 2021 16:46:44 -0800
Subject: [PATCH] PR tree-optimization/101807

	* match.pd (bool0 < bool1) -> (!bool0 & bool1): New optimization.
	* match.pd  (bool0 <= bool1) -> (!bool0 | bool1): New optimization.
	* match.pd  (bool0 > bool1) -> (bool0 & !bool1): New optimization.
	* match.pd  (bool0 >= bool1) -> (bool0 | !bool1): New optimization.
	* gcc.dg/tree-ssa/pr101807.c: testcase for this optimization.
---
 gcc/match.pd                             | 33 ++++++++++++++++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/pr101807.c | 33 ++++++++++++++++++++++++
 2 files changed, 66 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr101807.c

diff --git a/gcc/match.pd b/gcc/match.pd
index a319aefa808..50ddb6d9e8c 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4449,6 +4449,39 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
  )
 )
 
+/* (bool0 < bool1) -> (!bool0 & bool1) */
+(simplify
+ (lt truth_valued_p@0 truth_valued_p@1)
+ (if (TREE_CODE (type) == BOOLEAN_TYPE
+      && types_match (type, TREE_TYPE (@0))
+      && types_match (TREE_TYPE (@0), TREE_TYPE (@1)))
+  (bit_and (bit_not @0) @1)))
+
+/* (bool0 <= bool1) -> (!bool0 | bool1) */
+(simplify
+ (le truth_valued_p@0 truth_valued_p@1)
+ (if (TREE_CODE (type) == BOOLEAN_TYPE
+      && types_match (type, TREE_TYPE (@0))
+      && types_match (TREE_TYPE (@0), TREE_TYPE (@1)))
+  (bit_ior (bit_not @0) @1)))
+
+/* (bool0 > bool1) -> (bool0 & !bool1) */
+(simplify
+ (gt truth_valued_p@0 truth_valued_p@1)
+ (if (TREE_CODE (type) == BOOLEAN_TYPE
+      && types_match (type, TREE_TYPE (@0))
+      && types_match (TREE_TYPE (@0), TREE_TYPE (@1)))
+  (bit_and @0 (bit_not @1))))
+
+/* (bool0 >= bool1) -> (bool0 | !bool1) */
+(simplify
+ (ge truth_valued_p@0 truth_valued_p@1)
+ (if (TREE_CODE (type) == BOOLEAN_TYPE
+      && types_match (type, TREE_TYPE (@0))
+      && types_match (TREE_TYPE (@0), TREE_TYPE (@1)))
+  (bit_ior @0 (bit_not @1))))
+
+
 /* -(type)!A -> (type)A - 1.  */
 (simplify
  (negate (convert?:s (logical_inverted_value:s @0)))
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr101807.c b/gcc/testsuite/gcc.dg/tree-ssa/pr101807.c
new file mode 100644
index 00000000000..29012ca1fb1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr101807.c
@@ -0,0 +1,33 @@
+/* PR tree-optimization/101807 */
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-optimized" } */
+
+#include <stdbool.h>
+
+bool g(bool a, bool b)
+{
+    return (a < b);
+}
+
+bool h(bool a, bool b)
+{
+  return (a <= b);
+}
+
+bool i(bool a, bool b)
+{
+  return (a > b);
+}
+
+bool j(bool a, bool b)
+{
+  return (a >= b);
+}
+
+
+/* Make sure we have removed < and <= from "a < b" and "a <= b". */
+/* { dg-final { scan-tree-dump-not " < " "optimized" } } */
+/* { dg-final { scan-tree-dump-not " <= " "optimized" } } */
+/* Make sure we have removed > and >= from "a > b" and "a >= b". */
+/* { dg-final { scan-tree-dump-not " > " "optimized" } } */
+/* { dg-final { scan-tree-dump-not " >= " "optimized" } } */
\ No newline at end of file
-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH][WIP] PR tree-optimization/101808 Boolean comparison simplification
  2021-11-23 18:34 [PATCH][WIP] PR tree-optimization/101808 Boolean comparison simplification Navid Rahimi
@ 2021-11-23 19:14 ` Jeff Law
  2021-11-23 19:33   ` Andrew Pinski
  2021-11-23 19:42   ` Navid Rahimi
  0 siblings, 2 replies; 11+ messages in thread
From: Jeff Law @ 2021-11-23 19:14 UTC (permalink / raw)
  To: Navid Rahimi, Navid Rahimi via Gcc-patches



On 11/23/2021 11:34 AM, Navid Rahimi via Gcc-patches wrote:
> Hi GCC community,
>
> I wanted you take a quick look at this patch to solve this bug [1]. This is the code example for the optimization [2] which does include a link to proof of each different optimization.
>
> I think it should be possible to use simpler approach than what Andrew has used here [3].
>
> P.S. Tested and verified on Linux x86_64.
>
> 1) https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101808
> 2) https://compiler-explorer.com/z/Gc448eE3z
> 3) https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101808#c1
Don't those match.pd patterns make things worse?  We're taking a single 
expression evaluation (the conditional) and turning it into two logicals 
AFAICT.

For the !x expression, obviously if x is a  constant, then we can 
compute that at compile time and we're going from a single conditional 
to a single logical which is probably a win, but that's not the case 
with this patch AFAICT.

jeff

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH][WIP] PR tree-optimization/101808 Boolean comparison simplification
  2021-11-23 19:14 ` Jeff Law
@ 2021-11-23 19:33   ` Andrew Pinski
  2021-11-23 19:55     ` [EXTERNAL] " Navid Rahimi
  2021-11-23 19:42   ` Navid Rahimi
  1 sibling, 1 reply; 11+ messages in thread
From: Andrew Pinski @ 2021-11-23 19:33 UTC (permalink / raw)
  To: Jeff Law; +Cc: Navid Rahimi, Navid Rahimi via Gcc-patches

On Tue, Nov 23, 2021 at 11:15 AM Jeff Law via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
>
> On 11/23/2021 11:34 AM, Navid Rahimi via Gcc-patches wrote:
> > Hi GCC community,
> >
> > I wanted you take a quick look at this patch to solve this bug [1]. This is the code example for the optimization [2] which does include a link to proof of each different optimization.
> >
> > I think it should be possible to use simpler approach than what Andrew has used here [3].
> >
> > P.S. Tested and verified on Linux x86_64.
> >
> > 1) https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101808
> > 2) https://compiler-explorer.com/z/Gc448eE3z
> > 3) https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101808#c1
> Don't those match.pd patterns make things worse?  We're taking a single
> expression evaluation (the conditional) and turning it into two logicals
> AFAICT.
>
> For the !x expression, obviously if x is a  constant, then we can
> compute that at compile time and we're going from a single conditional
> to a single logical which is probably a win, but that's not the case
> with this patch AFAICT.

One thing is you could use ! to see if bit_not simplifies down to a
constant which is what I did in the bug report.  But it might be more
useful to use the ^ flag (which I added in a different patch) which
says the bit_xor is removed then accept it.

Note (bit_not @0) is wrong, it should be (bit_xor @0 { booleantrue; }
) as there are boolean types which are signed and/or > 1 precision
which is what I had in my patch.
Did you test Ada with this patch as that is where the "odd" boolean
types show up?

Thanks,
Andrew Pinski


>
> jeff

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [EXTERNAL] Re: [PATCH][WIP] PR tree-optimization/101808 Boolean comparison simplification
  2021-11-23 19:14 ` Jeff Law
  2021-11-23 19:33   ` Andrew Pinski
@ 2021-11-23 19:42   ` Navid Rahimi
  2021-11-23 20:02     ` Jeff Law
  1 sibling, 1 reply; 11+ messages in thread
From: Navid Rahimi @ 2021-11-23 19:42 UTC (permalink / raw)
  To: Jeff Law, Navid Rahimi via Gcc-patches

In case of x86_64. This is the code:

src_1(bool, bool):
        cmp     dil, sil
        setb    al
        ret

tgt_1(bool, bool):
        xor     edi, 1
        mov     eax, edi
        and     eax, esi
        ret


Lets look at the latency of the src_1:
cmp: latency of 1: (page 663, table C-17)
setb: latency of 2. They don't report setb latency in intel instruction manual. But the closest instruction to this setbe does have latency of 2.

But for tgt_1:
xor: latency 1.
mov: latency 1. (But it seems x86_64 does optimize this instruction and basically it is latency 0 in this case.  In Zero-Latency MOV Instructions section they explain it [1].)
and: latency 1.

So even if you consider setb as latency of 1 it is equal. But if it is latency of 2, it should be a 1 latency win.

1) https://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-optimization-manual.pdf

Best wishes,
Navid.

________________________________________
From: Jeff Law <jeffreyalaw@gmail.com>
Sent: Tuesday, November 23, 2021 11:14
To: Navid Rahimi; Navid Rahimi via Gcc-patches
Subject: [EXTERNAL] Re: [PATCH][WIP] PR tree-optimization/101808 Boolean comparison simplification



On 11/23/2021 11:34 AM, Navid Rahimi via Gcc-patches wrote:
> Hi GCC community,
>
> I wanted you take a quick look at this patch to solve this bug [1]. This is the code example for the optimization [2] which does include a link to proof of each different optimization.
>
> I think it should be possible to use simpler approach than what Andrew has used here [3].
>
> P.S. Tested and verified on Linux x86_64.
>
> 1) https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fbugzilla%2Fshow_bug.cgi%3Fid%3D101808&amp;data=04%7C01%7Cnavidrahimi%40microsoft.com%7C29308ca3ff234b91a31608d9aeb57500%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637732916650766903%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=m%2BIgviZpMo0MT369dcIzefp810oz%2FMU9LC1Mk2FdChk%3D&amp;reserved=0
> 2) https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcompiler-explorer.com%2Fz%2FGc448eE3z&amp;data=04%7C01%7Cnavidrahimi%40microsoft.com%7C29308ca3ff234b91a31608d9aeb57500%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637732916650766903%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=IwNQZsaEUaB1MKRfL8OWkWYvx0ODq86Obt3eFuxZD40%3D&amp;reserved=0
> 3) https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fbugzilla%2Fshow_bug.cgi%3Fid%3D101808%23c1&amp;data=04%7C01%7Cnavidrahimi%40microsoft.com%7C29308ca3ff234b91a31608d9aeb57500%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637732916650766903%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=2DB2AlZWbjJ6Fd3Aw%2Bb2Oub3t8d1i7%2FnaUQKUe2m4uQ%3D&amp;reserved=0
Don't those match.pd patterns make things worse?  We're taking a single
expression evaluation (the conditional) and turning it into two logicals
AFAICT.

For the !x expression, obviously if x is a  constant, then we can
compute that at compile time and we're going from a single conditional
to a single logical which is probably a win, but that's not the case
with this patch AFAICT.

jeff

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [EXTERNAL] Re: [PATCH][WIP] PR tree-optimization/101808 Boolean comparison simplification
  2021-11-23 19:33   ` Andrew Pinski
@ 2021-11-23 19:55     ` Navid Rahimi
  2021-11-23 20:03       ` Jeff Law
  0 siblings, 1 reply; 11+ messages in thread
From: Navid Rahimi @ 2021-11-23 19:55 UTC (permalink / raw)
  To: Andrew Pinski, Jeff Law; +Cc: Navid Rahimi via Gcc-patches

> Did you test Ada with this patch as that is where the "odd" boolean
> types show up?
No I haven't tested Ada yet. Since it is work in progress still [WIP]. Quick question, to prevent applying this optimization to those odd Boolean types in Ada, there should be a check to check whether it is canonical boolean type or signed/unsigned, which should prevent messing with odd Boolean types in Ada.

Best wishes,
Navid.

________________________________________
From: Andrew Pinski <pinskia@gmail.com>
Sent: Tuesday, November 23, 2021 11:33
To: Jeff Law
Cc: Navid Rahimi; Navid Rahimi via Gcc-patches
Subject: [EXTERNAL] Re: [PATCH][WIP] PR tree-optimization/101808 Boolean comparison simplification

[You don't often get email from pinskia@gmail.com. Learn why this is important at http://aka.ms/LearnAboutSenderIdentification.]

On Tue, Nov 23, 2021 at 11:15 AM Jeff Law via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
>
> On 11/23/2021 11:34 AM, Navid Rahimi via Gcc-patches wrote:
> > Hi GCC community,
> >
> > I wanted you take a quick look at this patch to solve this bug [1]. This is the code example for the optimization [2] which does include a link to proof of each different optimization.
> >
> > I think it should be possible to use simpler approach than what Andrew has used here [3].
> >
> > P.S. Tested and verified on Linux x86_64.
> >
> > 1) https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fbugzilla%2Fshow_bug.cgi%3Fid%3D101808&amp;data=04%7C01%7Cnavidrahimi%40microsoft.com%7C7b45fdd017874f287caf08d9aeb836ad%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637732928490579680%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=UsE0BbrZpRhrPZZreF%2Bj2spaYmJZuVLc053sWTFG6Ow%3D&amp;reserved=0
> > 2) https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcompiler-explorer.com%2Fz%2FGc448eE3z&amp;data=04%7C01%7Cnavidrahimi%40microsoft.com%7C7b45fdd017874f287caf08d9aeb836ad%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637732928490579680%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=vGlVXyBqBeABvP8hQGb6paYj1t078rSlLdpI0t6qDlc%3D&amp;reserved=0
> > 3) https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fbugzilla%2Fshow_bug.cgi%3Fid%3D101808%23c1&amp;data=04%7C01%7Cnavidrahimi%40microsoft.com%7C7b45fdd017874f287caf08d9aeb836ad%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637732928490579680%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=DNAyrNo9uZ05FyJYOdc%2BD85Dc9A3VgP95htSfaxRS40%3D&amp;reserved=0
> Don't those match.pd patterns make things worse?  We're taking a single
> expression evaluation (the conditional) and turning it into two logicals
> AFAICT.
>
> For the !x expression, obviously if x is a  constant, then we can
> compute that at compile time and we're going from a single conditional
> to a single logical which is probably a win, but that's not the case
> with this patch AFAICT.

One thing is you could use ! to see if bit_not simplifies down to a
constant which is what I did in the bug report.  But it might be more
useful to use the ^ flag (which I added in a different patch) which
says the bit_xor is removed then accept it.

Note (bit_not @0) is wrong, it should be (bit_xor @0 { booleantrue; }
) as there are boolean types which are signed and/or > 1 precision
which is what I had in my patch.
Did you test Ada with this patch as that is where the "odd" boolean
types show up?

Thanks,
Andrew Pinski


>
> jeff

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [EXTERNAL] Re: [PATCH][WIP] PR tree-optimization/101808 Boolean comparison simplification
  2021-11-23 19:42   ` Navid Rahimi
@ 2021-11-23 20:02     ` Jeff Law
  2021-11-23 20:08       ` Navid Rahimi
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff Law @ 2021-11-23 20:02 UTC (permalink / raw)
  To: Navid Rahimi, Navid Rahimi via Gcc-patches



On 11/23/2021 12:42 PM, Navid Rahimi wrote:
> In case of x86_64. This is the code:
>
> src_1(bool, bool):
>          cmp     dil, sil
>          setb    al
>          ret
>
> tgt_1(bool, bool):
>          xor     edi, 1
>          mov     eax, edi
>          and     eax, esi
>          ret
>
>
> Lets look at the latency of the src_1:
> cmp: latency of 1: (page 663, table C-17)
> setb: latency of 2. They don't report setb latency in intel instruction manual. But the closest instruction to this setbe does have latency of 2.
>
> But for tgt_1:
> xor: latency 1.
> mov: latency 1. (But it seems x86_64 does optimize this instruction and basically it is latency 0 in this case.  In Zero-Latency MOV Instructions section they explain it [1].)
> and: latency 1.
>
> So even if you consider setb as latency of 1 it is equal. But if it is latency of 2, it should be a 1 latency win.
>
> 1) https://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-optimization-manual.pdf
But these are target issues you've raised -- those should be handled in 
the RTL pipeline and are not a significant concern for gimple.

In gimple your primary goal should be to reduce the number of 
expressions that are evaluated.  This patch does the opposite.

jeff


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [EXTERNAL] Re: [PATCH][WIP] PR tree-optimization/101808 Boolean comparison simplification
  2021-11-23 19:55     ` [EXTERNAL] " Navid Rahimi
@ 2021-11-23 20:03       ` Jeff Law
  2021-11-29 23:51         ` Navid Rahimi
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff Law @ 2021-11-23 20:03 UTC (permalink / raw)
  To: Navid Rahimi, Andrew Pinski; +Cc: Navid Rahimi via Gcc-patches



On 11/23/2021 12:55 PM, Navid Rahimi wrote:
>> Did you test Ada with this patch as that is where the "odd" boolean
>> types show up?
> No I haven't tested Ada yet. Since it is work in progress still [WIP]. Quick question, to prevent applying this optimization to those odd Boolean types in Ada, there should be a check to check whether it is canonical boolean type or signed/unsigned, which should prevent messing with odd Boolean types in Ada.
IIRC, you should check the type's precision.  THere should be examples 
you can find in one or more of the gimple optimizers.

jeff


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [EXTERNAL] Re: [PATCH][WIP] PR tree-optimization/101808 Boolean comparison simplification
  2021-11-23 20:02     ` Jeff Law
@ 2021-11-23 20:08       ` Navid Rahimi
  2021-11-23 20:14         ` Jeff Law
  0 siblings, 1 reply; 11+ messages in thread
From: Navid Rahimi @ 2021-11-23 20:08 UTC (permalink / raw)
  To: Jeff Law, Navid Rahimi via Gcc-patches

> In gimple your primary goal should be to reduce the number of
> expressions that are evaluated.  This patch does the opposite.

That is actually a really good point in my opinion. I am hesitant about this patch and wanted to hear gcc-patch opinion about this. Doing something like this in IR level is a little bit counter intuitive to me. I will take a look at LLVM in my spare time to see where they are transferring that pattern and what was the rationale behind it.

Best wishes,
Navid.

________________________________________
From: Jeff Law <jeffreyalaw@gmail.com>
Sent: Tuesday, November 23, 2021 12:02
To: Navid Rahimi; Navid Rahimi via Gcc-patches
Subject: Re: [EXTERNAL] Re: [PATCH][WIP] PR tree-optimization/101808 Boolean comparison simplification



On 11/23/2021 12:42 PM, Navid Rahimi wrote:
> In case of x86_64. This is the code:
>
> src_1(bool, bool):
>          cmp     dil, sil
>          setb    al
>          ret
>
> tgt_1(bool, bool):
>          xor     edi, 1
>          mov     eax, edi
>          and     eax, esi
>          ret
>
>
> Lets look at the latency of the src_1:
> cmp: latency of 1: (page 663, table C-17)
> setb: latency of 2. They don't report setb latency in intel instruction manual. But the closest instruction to this setbe does have latency of 2.
>
> But for tgt_1:
> xor: latency 1.
> mov: latency 1. (But it seems x86_64 does optimize this instruction and basically it is latency 0 in this case.  In Zero-Latency MOV Instructions section they explain it [1].)
> and: latency 1.
>
> So even if you consider setb as latency of 1 it is equal. But if it is latency of 2, it should be a 1 latency win.
>
> 1) https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.intel.com%2Fcontent%2Fdam%2Fwww%2Fpublic%2Fus%2Fen%2Fdocuments%2Fmanuals%2F64-ia-32-architectures-optimization-manual.pdf&amp;data=04%7C01%7Cnavidrahimi%40microsoft.com%7Cda4bfe80ceaa432a813e08d9aebc33ee%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637732945624565576%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=sopToDx8Y4xfROdI7nRYxYQ%2FCHJPgjIKKGEaWiAXmL4%3D&amp;reserved=0
But these are target issues you've raised -- those should be handled in
the RTL pipeline and are not a significant concern for gimple.

In gimple your primary goal should be to reduce the number of
expressions that are evaluated.  This patch does the opposite.

jeff


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [EXTERNAL] Re: [PATCH][WIP] PR tree-optimization/101808 Boolean comparison simplification
  2021-11-23 20:08       ` Navid Rahimi
@ 2021-11-23 20:14         ` Jeff Law
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff Law @ 2021-11-23 20:14 UTC (permalink / raw)
  To: Navid Rahimi, Navid Rahimi via Gcc-patches



On 11/23/2021 1:08 PM, Navid Rahimi wrote:
>> In gimple your primary goal should be to reduce the number of
>> expressions that are evaluated.  This patch does the opposite.
> That is actually a really good point in my opinion. I am hesitant about this patch and wanted to hear gcc-patch opinion about this. Doing something like this in IR level is a little bit counter intuitive to me. I will take a look at LLVM in my spare time to see where they are transferring that pattern and what was the rationale behind it.
It could be easily looked at as target expansion issue.  ie, there's two 
equivalent forms for the full expression and the desired form varies 
based on some property of the target.  The idea we've kicked around, but 
not implemented, would be to allow target specific match.pd patterns to 
drive rewriting expressions at the gimple->rtl border.

Jeff


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [EXTERNAL] Re: [PATCH][WIP] PR tree-optimization/101808 Boolean comparison simplification
  2021-11-23 20:03       ` Jeff Law
@ 2021-11-29 23:51         ` Navid Rahimi
  2021-12-03 15:43           ` Jeff Law
  0 siblings, 1 reply; 11+ messages in thread
From: Navid Rahimi @ 2021-11-29 23:51 UTC (permalink / raw)
  To: Jeff Law, Andrew Pinski; +Cc: Navid Rahimi via Gcc-patches

Jeff,

Sorry for bringing back this thread.

Quick question, you mentioned checking the TYPE_PRECISION to make sure the type is a canonical Boolean type (and not a fancy signed/unsigned Boolean type from Ada Andrew mentioned).
But I noticed that truth_valued_p does already check for:
(if (INTEGRAL_TYPE_P (type) && TYPE_PRECISION (type) == 1))

So in this case, there should not any other Boolean type fall into truth_valued_p type [1]? Is that right?

1) https://github.com/gcc-mirror/gcc/blob/master/gcc/match.pd#L1717

Best wishes,
Navid.

________________________________________
From: Jeff Law <jeffreyalaw@gmail.com>
Sent: Tuesday, November 23, 2021 12:03
To: Navid Rahimi; Andrew Pinski
Cc: Navid Rahimi via Gcc-patches
Subject: Re: [EXTERNAL] Re: [PATCH][WIP] PR tree-optimization/101808 Boolean comparison simplification



On 11/23/2021 12:55 PM, Navid Rahimi wrote:
>> Did you test Ada with this patch as that is where the "odd" boolean
>> types show up?
> No I haven't tested Ada yet. Since it is work in progress still [WIP]. Quick question, to prevent applying this optimization to those odd Boolean types in Ada, there should be a check to check whether it is canonical boolean type or signed/unsigned, which should prevent messing with odd Boolean types in Ada.
IIRC, you should check the type's precision.  THere should be examples
you can find in one or more of the gimple optimizers.

jeff


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [EXTERNAL] Re: [PATCH][WIP] PR tree-optimization/101808 Boolean comparison simplification
  2021-11-29 23:51         ` Navid Rahimi
@ 2021-12-03 15:43           ` Jeff Law
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff Law @ 2021-12-03 15:43 UTC (permalink / raw)
  To: Navid Rahimi, Andrew Pinski; +Cc: Navid Rahimi via Gcc-patches



On 11/29/2021 4:51 PM, Navid Rahimi wrote:
> Jeff,
>
> Sorry for bringing back this thread.
>
> Quick question, you mentioned checking the TYPE_PRECISION to make sure the type is a canonical Boolean type (and not a fancy signed/unsigned Boolean type from Ada Andrew mentioned).
> But I noticed that truth_valued_p does already check for:
> (if (INTEGRAL_TYPE_P (type) && TYPE_PRECISION (type) == 1))
>
> So in this case, there should not any other Boolean type fall into truth_valued_p type [1]? Is that right?
Yes, correct.

Jeff


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-12-03 15:43 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-23 18:34 [PATCH][WIP] PR tree-optimization/101808 Boolean comparison simplification Navid Rahimi
2021-11-23 19:14 ` Jeff Law
2021-11-23 19:33   ` Andrew Pinski
2021-11-23 19:55     ` [EXTERNAL] " Navid Rahimi
2021-11-23 20:03       ` Jeff Law
2021-11-29 23:51         ` Navid Rahimi
2021-12-03 15:43           ` Jeff Law
2021-11-23 19:42   ` Navid Rahimi
2021-11-23 20:02     ` Jeff Law
2021-11-23 20:08       ` Navid Rahimi
2021-11-23 20:14         ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).