public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libiberty: d-demangle: use switch instead of if-else
@ 2021-09-30  1:08 Luís Ferreira
  2021-10-03 21:55 ` Jeff Law
  0 siblings, 1 reply; 4+ messages in thread
From: Luís Ferreira @ 2021-09-30  1:08 UTC (permalink / raw)
  To: gcc-patches; +Cc: Luís Ferreira

This patch allows the compiler to efficiently generate jump tables instead of
using if-else-if.

Signed-off-by: Luís Ferreira <contact@lsferreira.net>
---
 libiberty/d-demangle.c | 44 +++++++++++++++++++-----------------------
 1 file changed, 20 insertions(+), 24 deletions(-)

diff --git a/libiberty/d-demangle.c b/libiberty/d-demangle.c
index 3adf7b562d1..9748a925371 100644
--- a/libiberty/d-demangle.c
+++ b/libiberty/d-demangle.c
@@ -774,30 +774,26 @@ dlang_type (string *decl, const char *mangled, struct dlang_info *info)
       return mangled;
     case 'N':
       mangled++;
-      if (*mangled == 'g') /* wild(T) */
-	{
-	  mangled++;
-	  string_append (decl, "inout(");
-	  mangled = dlang_type (decl, mangled, info);
-	  string_append (decl, ")");
-	  return mangled;
-	}
-      else if (*mangled == 'h') /* vector(T) */
-	{
-	  mangled++;
-	  string_append (decl, "__vector(");
-	  mangled = dlang_type (decl, mangled, info);
-	  string_append (decl, ")");
-	  return mangled;
-	}
-      else if (*mangled == 'n') /* typeof(*null) */
-	{
-	  mangled++;
-	  string_append (decl, "typeof(*null)");
-	  return mangled;
-	}
-      else
-	return NULL;
+      switch (*mangled)
+      {
+	case 'g': /* wild(T) */
+	    mangled++;
+	    string_append (decl, "inout(");
+	    mangled = dlang_type (decl, mangled, info);
+	    string_append (decl, ")");
+	    return mangled;
+	case 'h': /* vector(T) */
+	    mangled++;
+	    string_append (decl, "__vector(");
+	    mangled = dlang_type (decl, mangled, info);
+	    string_append (decl, ")");
+	    return mangled;
+	case 'n': /* typeof(*null) */
+	    mangled++;
+	    string_append (decl, "typeof(*null)");
+	    return mangled;
+      }
+      return NULL;
     case 'A': /* dynamic array (T[]) */
       mangled++;
       mangled = dlang_type (decl, mangled, info);
-- 
2.33.0


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

* Re: [PATCH] libiberty: d-demangle: use switch instead of if-else
  2021-09-30  1:08 [PATCH] libiberty: d-demangle: use switch instead of if-else Luís Ferreira
@ 2021-10-03 21:55 ` Jeff Law
  2021-10-04  7:25   ` ibuclaw
  2021-10-04 14:56   ` Luís Ferreira
  0 siblings, 2 replies; 4+ messages in thread
From: Jeff Law @ 2021-10-03 21:55 UTC (permalink / raw)
  To: Luís Ferreira, gcc-patches



On 9/29/2021 7:08 PM, Luís Ferreira wrote:
> This patch allows the compiler to efficiently generate jump tables instead of
> using if-else-if.
>
> Signed-off-by: Luís Ferreira <contact@lsferreira.net>
I'm not sure this is terribly useful.  Compilers have the ability to 
analyze the underlying code and make sensible decisions for how to 
implement either form.   So the right metric here is does this make the 
code cleaner/easier to understand.  With just 3 clauses it's hard (for 
me) to make the case that it is considerably cleaner.

Jeff



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

* Re: [PATCH] libiberty: d-demangle: use switch instead of if-else
  2021-10-03 21:55 ` Jeff Law
@ 2021-10-04  7:25   ` ibuclaw
  2021-10-04 14:56   ` Luís Ferreira
  1 sibling, 0 replies; 4+ messages in thread
From: ibuclaw @ 2021-10-04  7:25 UTC (permalink / raw)
  To: Jeff Law, Jeff Law via Gcc-patches, Luís Ferreira

> On 03/10/2021 23:55 Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
>  
> On 9/29/2021 7:08 PM, Luís Ferreira wrote:
> > This patch allows the compiler to efficiently generate jump tables instead of
> > using if-else-if.
> >
> > Signed-off-by: Luís Ferreira <contact@lsferreira.net>
> I'm not sure this is terribly useful.  Compilers have the ability to 
> analyze the underlying code and make sensible decisions for how to 
> implement either form.   So the right metric here is does this make the 
> code cleaner/easier to understand.  With just 3 clauses it's hard (for 
> me) to make the case that it is considerably cleaner.
> 

I'd be inclined to agree here.

FAOD, I put together a quick example of what difference this patch makes.  Other than freely reordering the conditions, the answer is nothing.

https://godbolt.org/z/nKjjv64zM

Iain.

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

* Re: [PATCH] libiberty: d-demangle: use switch instead of if-else
  2021-10-03 21:55 ` Jeff Law
  2021-10-04  7:25   ` ibuclaw
@ 2021-10-04 14:56   ` Luís Ferreira
  1 sibling, 0 replies; 4+ messages in thread
From: Luís Ferreira @ 2021-10-04 14:56 UTC (permalink / raw)
  To: Jeff Law, gcc-patches

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

On Sun, 2021-10-03 at 15:55 -0600, Jeff Law wrote:
> 
> 
> On 9/29/2021 7:08 PM, Luís Ferreira wrote:
> > This patch allows the compiler to efficiently generate jump tables
> > instead of
> > using if-else-if.
> > 
> > Signed-off-by: Luís Ferreira <contact@lsferreira.net>
> I'm not sure this is terribly useful.  Compilers have the ability to 
> analyze the underlying code and make sensible decisions for how to 
> implement either form.   So the right metric here is does this make the
> code cleaner/easier to understand.  With just 3 clauses it's hard (for 
> me) to make the case that it is considerably cleaner.
> 
> Jeff
> 
> 

Well, my major point on the patch was performance, eventhough on this
number of clauses it doesn't matter for GCC. I can totally agree with
you.

Although when we talk about a higher number of cases the optimizer
reach a threashold to optimize with jump tables and if-else-if starts
to loose there. Furthermore, if-else-if are hard to optimize, since the
compiler need to check if the condition order is important and with a
high number of cases the compiler may just give up on that check. I'm
not particularly aware of how GCC theoretically optimize it, so take
that with a grain of salt, but, in practice, it sure have a difference.
https://godbolt.org/z/rT9drW117

Even though performance may not be terribly bad (some compilers may not
be really that smart, specially in higher number of cases, as shown
above) I can still consider this change for future additions, even
though debatable, since mangling characters may exapand this logic and
increase the number of cases making if-else-if code a bit longer to
read.

Overall, although, this is by far a minor thing. My intention with this
was to reflect some changes I found relevant while reading this file.

Side note:

This was a previously sent patch. As requested by you, if this is
somehow being considered to be merged, here is the changelog:

ChangeLog:

libiberty/
	* d-demangle.c (dlang_type): Change if-else-if to switch case.

-- 
Sincerely,
Luís Ferreira @ lsferreira.net


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

end of thread, other threads:[~2021-10-04 14:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-30  1:08 [PATCH] libiberty: d-demangle: use switch instead of if-else Luís Ferreira
2021-10-03 21:55 ` Jeff Law
2021-10-04  7:25   ` ibuclaw
2021-10-04 14:56   ` Luís Ferreira

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).