diff mbox series

libiberty: d-demangle: use switch instead of if-else

Message ID 20210930010804.750229-1-contact@lsferreira.net
State New
Headers show
Series libiberty: d-demangle: use switch instead of if-else | expand

Commit Message

Luís Ferreira Sept. 30, 2021, 1:08 a.m. UTC
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(-)

Comments

Jeff Law Oct. 3, 2021, 9:55 p.m. UTC | #1
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
Bill Schmidt via Gcc-patches Oct. 4, 2021, 7:25 a.m. UTC | #2
> 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.
Luís Ferreira Oct. 4, 2021, 2:56 p.m. UTC | #3
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.
diff mbox series

Patch

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