libiberty: d-demangle: use switch instead of if-else
Commit Message
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
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
> 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.
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.
@@ -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);